9 files changed, 131 insertions, 54 deletions

diff --git a/tests/integration/harness.py b/tests/integration/harness.py
index e5ec621..f3cd0fa 100644
--- a/tests/integration/harness.py
+++ b/tests/integration/harness.py
@@ -135,6 +135,11 @@ class Tracker:
         self.log.seek(0)
         return self.log.read().decode("utf-8", "replace")
 
+    def reload(self):
+        """SIGHUP makes the accesslist reader re-read its files, which also
+           empties the dynamic add/delete changesets."""
+        os.kill(self.process.pid, signal.SIGHUP)
+
     def stop(self):
         """Returns anything the tracker complained about on the way out.
 
diff --git a/tests/integration/test_accesslist.py b/tests/integration/test_accesslist.py
index 7d01264..3ed8ddf 100644
--- a/tests/integration/test_accesslist.py
+++ b/tests/integration/test_accesslist.py
@@ -7,6 +7,7 @@ produces: an empty file, and a last line with no newline after it.
 
 import os
 import struct
+import time
 import unittest
 
 from harness import Tracker, TrackerTestCase
@@ -109,9 +110,10 @@ class ListIsInPlaceBeforeTheStateFile(TrackerTestCase):
     files = {"blacklist": listing(BLOCKED)}
 
     def test_a_blacklisted_torrent_in_a_state_file_is_not_restored(self):
+        touched = int(time.time()) // 60 - 5
         path = os.path.join(self.tracker.dir, "state")
         with open(path, "w") as handle:
-            handle.write("%s:5:7\n%s:5:7\n" % (BLOCKED.hex(), ALLOWED.hex()))
+            handle.write("%s:%d:7\n%s:%d:7\n" % (BLOCKED.hex(), touched, ALLOWED.hex(), touched))
 
         restarted = Tracker(self.config, self.files, extra_args=["-l", path])
         try:
diff --git a/tests/integration/test_dynamic_accesslist.py b/tests/integration/test_dynamic_accesslist.py
index 7fa5fde..2072f5e 100644
--- a/tests/integration/test_dynamic_accesslist.py
+++ b/tests/integration/test_dynamic_accesslist.py
@@ -40,6 +40,24 @@ class DynamicAccesslist(TrackerTestCase):
             time.sleep(0.05)
         return False
 
+    def test_a_hash_added_after_a_reload_is_blocked(self):
+        """A reload leaves the add changeset an empty list rather than an
+        absent one. Adding against it walked a null array; the hash must be
+        blocked and the tracker must stay up."""
+        OTHER = bytes(range(21, 41))
+        self.feed("add.fifo", BLOCKED)
+        self.assertTrue(self.wait_for(BLOCKED, blocked=True), "the first hash was never blocked")
+
+        # The reload empties the add changeset, which unblocks the first hash.
+        # Wait for that before adding the second, so it lands on the empty list
+        # the bug walked, not on the changeset before the reload has cleared it.
+        self.tracker.reload()
+        self.assertTrue(self.wait_for(BLOCKED, blocked=False), "the reload never emptied the changeset")
+
+        self.feed("add.fifo", OTHER)
+        self.assertTrue(self.wait_for(OTHER, blocked=True), "the hash added after the reload was never blocked")
+        # tearDown's stop() surfaces any sanitizer complaint on the way out.
+
     def test_a_hash_can_be_blocked_and_released_while_running(self):
         self.assertNotIn(b"failure reason", self.tracker.announce(BLOCKED))
 
diff --git a/tests/integration/test_fullscrape.py b/tests/integration/test_fullscrape.py
index 3150612..45b3ffb 100644
--- a/tests/integration/test_fullscrape.py
+++ b/tests/integration/test_fullscrape.py
@@ -8,6 +8,7 @@ so these also exercise the chunked transfer the ordinary replies never take.
 
 import gzip
 import struct
+import time
 import unittest
 
 from harness import TrackerTestCase, bdecode
@@ -78,15 +79,14 @@ class FullScrape(TrackerTestCase):
             escaped = line.rsplit(b":", 2)[0]
             self.assertIn(b"%", escaped, "these hashes are mostly unprintable bytes")
 
-    def test_the_state_dump_carries_an_age_rather_than_a_peer_count(self):
-        """What a restart reads back. The second column is how many minutes
-        ago the torrent was last touched, not a stamp and not a count of
-        anybody: the clock counts from boot, so an absolute time would mean
-        nothing to the process that reads it."""
+    def test_the_state_dump_carries_a_stamp_rather_than_a_peer_count(self):
+        """What a restart reads back. The second column is the minute the
+        torrent was last touched and not a count of anybody, by the wall
+        clock, which is the only one two runs of the tracker agree on."""
         entries = self.entries_from_lines(self.scrape_all("/stats?mode=statedump"))
         self.assertIn(self.seeding.hex().upper().encode(), entries)
-        age, completed = entries[self.seeding.hex().upper().encode()]
-        self.assertEqual(int(age), 0, "announced a moment ago")
+        stamp, completed = entries[self.seeding.hex().upper().encode()]
+        self.assertAlmostEqual(int(stamp), int(time.time()) // 60, delta=1, msg="announced a moment ago")
         self.assertEqual(int(completed), 0)
 
     # ------------------------------------------------------------ shape
diff --git a/tests/integration/test_keepalive.py b/tests/integration/test_keepalive.py
index 2c724fc..f9aa468 100644
--- a/tests/integration/test_keepalive.py
+++ b/tests/integration/test_keepalive.py
@@ -107,6 +107,22 @@ class KeepAlive(TrackerTestCase):
         finally:
             c.close()
 
+    def test_a_tiny_split_request_does_not_overread(self):
+        """A request that arrives in pieces and completes as a buffer shorter
+        than the dispatch literals (\"GET /\" etc.) used to read past the
+        exactly sized buffer. It must be rejected without a sanitizer hit."""
+        c = self.tracker.connect()
+        try:
+            c.sock.sendall(b"\r\n")
+            time.sleep(0.05)
+            c.sock.sendall(b"\r\n")          # completes as a 4-byte buffer
+            time.sleep(0.1)
+        finally:
+            c.close()
+        # the tracker must still answer; tearDown's stop() surfaces any sanitizer abort
+        answer = self.tracker.announce(self.hash_for("after-tiny"), port=6881, left=0)
+        self.assertNotIn(b"failure reason", answer, "the tracker stopped serving after a tiny request")
+
     def test_a_split_request_answered_by_a_worker_does_not_spin(self):
         """A reply that a worker produces does not go through the send path,
         so the request stays in the buffer the read loop is reading. The loop
diff --git a/tests/integration/test_livesync.py b/tests/integration/test_livesync.py
index f4cb2db..e146d93 100644
--- a/tests/integration/test_livesync.py
+++ b/tests/integration/test_livesync.py
@@ -72,6 +72,19 @@ class LiveSyncIn(TrackerTestCase):
         self.assertTrue(self.tracker.wait_until(lambda: self.counts(info_hash) == (1, 0)),
                         "scrape says %r" % (self.counts(info_hash),))
 
+    def test_many_v6_announces_do_not_overflow_the_send_buffer(self):
+        """The outgoing peer buffer is filled by announces to a clustered
+        tracker. The flush watermark used sizeof(ot_peer) (== 1), so a v6
+        record could be appended past the 1480-byte buffer onto the adjacent
+        fill field, which then drove an oversized stack copy. Well past the
+        old overflow point (~37 v6 peers) the tracker must still be alive."""
+        last = None
+        for i in range(80):
+            last = self.tracker.announce(self.hash_for("flood-%d" % i),
+                                         ip="2001:db8::%x" % (i + 1), port=7000 + i, left=1)
+        self.assertIn(b"complete", last, "the tracker stopped answering mid-flood")
+        # tearDown's stop() surfaces any sanitizer complaint on the way out.
+
     def test_one_packet_carries_several_torrents(self):
         hashes = [self.hash_for("sync-many-%d" % i) for i in range(4)]
         self.tracker.livesync_send(packet(SYNC_PEER4, [(h, peer4("10.1.2.%d" % (5 + i), 6881, FLAG_SEEDING))
diff --git a/tests/integration/test_statefile.py b/tests/integration/test_statefile.py
index 21eefae..a3c8d59 100644
--- a/tests/integration/test_statefile.py
+++ b/tests/integration/test_statefile.py
@@ -1,12 +1,12 @@
 """The state dump and the -l switch that reads it back.
 
-Nothing exercised the load path before, and the dump's second column is now
-how long ago a torrent was last touched rather than when, because the clock
-counts from boot and an absolute stamp would mean nothing to the process
-that reads the file.
+The second column is the minute a torrent was last touched, by the wall
+clock, which is what the file has recorded for years and the only clock two
+runs of the tracker agree on.
 """
 
 import os
+import time
 import unittest
 
 from harness import Tracker, TrackerTestCase
@@ -54,23 +54,26 @@ class StateFileRoundTrip(TrackerTestCase):
             restarted._cleanup()
 
 
-class StateFileWithNonsenseAges(TrackerTestCase):
-    """The age column is read as unsigned and handed on as a signed ot_time.
-    A value at or above 2^63 arrived negative, cleared the timeout check, and
+class StateFileWithNonsenseStamps(TrackerTestCase):
+    """The column is read as unsigned and handed on as a signed ot_time. A
+    value at or above 2^63 arrived negative, cleared the timeout check, and
     put the torrent's base in the future, where the cleaner never reaches it.
     At exactly 2^63 the subtraction itself overflowed."""
 
-    AGES = {
-        "overflow": 2**63,
-        "wrapped": 2**63 + 60 * 24,
-        "minus-one": 2**64 - 1,
-        "int64-max": 2**63 - 1,
-        "past-timeout": 60 * 24 + 1,
-    }
+    def stamps(self):
+        now = int(time.time()) // 60
+        return {
+            "overflow": 2**63,
+            "minus-one": 2**64 - 1,
+            "int64-max": 2**63 - 1,
+            "in-the-future": now + 60,
+            "past-timeout": now - (24 * 60 + 1),
+        }
 
-    def test_no_age_outside_the_timeout_builds_a_torrent(self):
-        lines = ["%s:%d:7" % (self.hash_for(name).hex(), age) for name, age in self.AGES.items()]
-        lines.append("%s:5:7" % self.hash_for("honest").hex())
+    def test_no_stamp_outside_the_timeout_builds_a_torrent(self):
+        stamps = self.stamps()
+        lines = ["%s:%d:7" % (self.hash_for(name).hex(), stamp) for name, stamp in stamps.items()]
+        lines.append("%s:%d:7" % (self.hash_for("honest").hex(), int(time.time()) // 60 - 5))
 
         path = os.path.join(self.tracker.dir, "nonsense")
         with open(path, "w") as handle:
@@ -78,9 +81,9 @@ class StateFileWithNonsenseAges(TrackerTestCase):
 
         restarted = Tracker(extra_args=["-l", path])
         try:
-            for name in self.AGES:
+            for name, stamp in stamps.items():
                 self.assertNotIn(self.hash_for(name), restarted.scrape(self.hash_for(name))[b"files"],
-                                 "the line with age %d was loaded" % self.AGES[name])
+                                 "the line stamped %d was loaded" % stamp)
             self.assertIn(self.hash_for("honest"), restarted.scrape(self.hash_for("honest"))[b"files"],
                           "the state file was not read at all")
             self.assertEqual(restarted.torrent_count(), 1)
diff --git a/tests/t_clean.c b/tests/t_clean.c
index 7eca9aa..2fadd03 100644
--- a/tests/t_clean.c
+++ b/tests/t_clean.c
@@ -11,7 +11,7 @@
 #include "trackerlogic.h"
 #include "test.h"
 
-/* A clock stepped backwards used to leave peer_list->base in the future, so
+/* A clock stepped backwards used to leave the torrent's base in the future, so
    the span the cleaner works from came out negative. It was then added to a
    peer's age and stored in the single byte that age lives in, where -1 reads
    as 255 and the next pass threw the whole swarm away. */
@@ -60,14 +60,12 @@ static ot_torrent *torrent_of(ot_hash *hash) {
 
 /* What a clock stepped back by `minutes` leaves behind */
 static void step_clock_back(ot_torrent *torrent, int minutes) {
-  torrent->peer_list6->base += minutes;
-  torrent->peer_list4->base += minutes;
+  torrent->base += minutes;
 }
 
 /* What the next minute of a well behaved clock looks like */
 static void age_by_a_minute(ot_torrent *torrent) {
-  torrent->peer_list6->base -= 1;
-  torrent->peer_list4->base -= 1;
+  torrent->base -= 1;
 }
 
 static void case_a_clock_that_went_back_keeps_the_swarm(void) {
diff --git a/tests/t_urlencoded.c b/tests/t_urlencoded.c
index 3b3e6c2..9935c1c 100644
--- a/tests/t_urlencoded.c
+++ b/tests/t_urlencoded.c
@@ -21,7 +21,7 @@ static void case_path(void) {
   ssize_t n;
 
   test_case("path stops at the question mark");
-  n = urlencoded_parse(&p, dest, SCAN_PATH);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_PATH);
   CHECK_INT(n, 9);
   dest[n] = 0;
   CHECK_STR(dest, "/announce");
@@ -35,22 +35,22 @@ static void case_param_and_value(void) {
   ssize_t n;
 
   test_case("parameter stops at '=', value stops at '&'");
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_PARAM);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_PARAM);
   CHECK_INT(n, 9);
   dest[n] = 0;
   CHECK_STR(dest, "info_hash");
 
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, 3);
   dest[n] = 0;
   CHECK_STR(dest, "abc");
 
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_PARAM);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_PARAM);
   CHECK_INT(n, 4);
   dest[n] = 0;
   CHECK_STR(dest, "port");
 
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, 2);
   dest[n] = 0;
   CHECK_STR(dest, "80");
@@ -63,7 +63,7 @@ static void case_percent_decoding(void) {
   ssize_t n;
 
   test_case("percent escapes decode");
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, 3);
   CHECK_MEM(dest, "ABC", 3);
 }
@@ -77,7 +77,7 @@ static void case_binary_info_hash(void) {
   ssize_t       n;
 
   test_case("twenty byte binary value survives decoding");
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, 20);
   CHECK_MEM(dest, want, 20);
 }
@@ -88,7 +88,7 @@ static void case_in_place(void) {
   ssize_t n;
 
   test_case("decoding in place over the source is safe");
-  n = urlencoded_parse(&p, query, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, query, query + sizeof(query), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, 2);
   CHECK_MEM(query, "AB", 2);
 }
@@ -100,7 +100,7 @@ static void case_bad_escape(void) {
   ssize_t n;
 
   test_case("a non hexadecimal escape is a parse error");
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, -1);
 }
 
@@ -111,7 +111,7 @@ static void case_truncated_escape(void) {
   ssize_t n;
 
   test_case("an escape cut short by the terminator is a parse error");
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, -1);
 }
 
@@ -123,11 +123,11 @@ static void case_terminator(void) {
   ssize_t n;
 
   test_case("a hard terminator before any output reports end of input");
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_PARAM);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_PARAM);
   CHECK_INT(n, -2);
 
   p = space;
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_PARAM);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_PARAM);
   CHECK_INT(n, -2);
 }
 
@@ -138,10 +138,10 @@ static void case_terminator_after_output(void) {
   ssize_t n;
 
   test_case("a hard terminator after output is left for the next call");
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, 3);
   CHECK_INT(*p, 0);
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, -2);
 }
 
@@ -157,14 +157,14 @@ static void case_separators_are_contextual(void) {
   ssize_t n;
 
   test_case("a question mark is literal inside a value");
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, 3);
   dest[n] = 0;
   CHECK_STR(dest, "a?b");
 
   test_case("an equals sign is literal inside a value");
   p = equals;
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, 3);
   dest[n] = 0;
   CHECK_STR(dest, "a=b");
@@ -177,7 +177,7 @@ static void case_ampersand_in_path(void) {
   ssize_t n;
 
   test_case("an ampersand in a path is a parse error");
-  n = urlencoded_parse(&p, dest, SCAN_PATH);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_PATH);
   CHECK_INT(n, -1);
 }
 
@@ -188,7 +188,7 @@ static void case_raw_high_byte(void) {
   ssize_t n;
 
   test_case("an undecoded byte above ASCII is a parse error");
-  n = urlencoded_parse(&p, dest, SCAN_SEARCHPATH_VALUE);
+  n = urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(n, -1);
 }
 
@@ -215,15 +215,15 @@ static void case_find_keywords(void) {
   char                    *p          = match;
 
   test_case("a known keyword returns its value");
-  CHECK_INT(urlencoded_match_keyword(keywords, &p, SCAN_SEARCHPATH_PARAM), 2);
+  CHECK_INT(urlencoded_match_keyword(keywords, &p, match + sizeof(match), SCAN_SEARCHPATH_PARAM), 2);
 
   test_case("an unknown keyword returns -3");
   p = nomatch;
-  CHECK_INT(urlencoded_match_keyword(keywords, &p, SCAN_SEARCHPATH_PARAM), -3);
+  CHECK_INT(urlencoded_match_keyword(keywords, &p, nomatch + sizeof(nomatch), SCAN_SEARCHPATH_PARAM), -3);
 
   test_case("a prefix of a keyword does not match it");
   p = prefix;
-  CHECK_INT(urlencoded_match_keyword(keywords, &p, SCAN_SEARCHPATH_PARAM), -3);
+  CHECK_INT(urlencoded_match_keyword(keywords, &p, prefix + sizeof(prefix), SCAN_SEARCHPATH_PARAM), -3);
 }
 
 /* The encoding direction. Which bytes survive unescaped is fixed by what
@@ -313,10 +313,30 @@ static void case_fmt_decoder_disagrees_on_dollar(void) {
   dest[n] = 0;
   CHECK_STR(dest, "a$b");
 
-  got = urlencoded_parse(&p, scanned, SCAN_SEARCHPATH_VALUE);
+  got = urlencoded_parse(&p, scanned, scanned + sizeof(scanned), SCAN_SEARCHPATH_VALUE);
   CHECK_INT(got, -1);
 }
 
+static void case_value_stops_at_dest_end(void) {
+  /* A value that decodes to more than the destination holds is a parse error,
+     not a write past the buffer. (ip= used to overflow ws->outbuf this way.) */
+  char  query[] = "AAAAAAAAAA&x=1";   /* ten A's then a terminator */
+  char  dest[4];
+  char *p = query;
+  test_case("a value longer than the destination returns -1");
+  CHECK_INT((int)urlencoded_parse(&p, dest, dest + sizeof(dest), SCAN_SEARCHPATH_VALUE), -1);
+}
+
+static void case_keyword_with_decoded_nul_does_not_match(void) {
+  /* A percent-encoded NUL used to make a value match a shorter keyword and
+     then index the keyword past its end. */
+  static const ot_keywords keywords[] = {{"port", 1}, {NULL, -3}};
+  char  query[] = "port%00xxxx=1";
+  char *p = query;
+  test_case("a keyword followed by a decoded NUL does not match the keyword");
+  CHECK_INT(urlencoded_match_keyword(keywords, &p, query + sizeof(query), SCAN_SEARCHPATH_PARAM), -3);
+}
+
 void test_urlencoded(void) {
   case_path();
   case_param_and_value();
@@ -332,6 +352,8 @@ void test_urlencoded(void) {
   case_raw_high_byte();
   case_skipvalue();
   case_find_keywords();
+  case_value_stops_at_dest_end();
+  case_keyword_with_decoded_nul_does_not_match();
   case_fmt_passes_safe_bytes();
   case_fmt_escapes_the_rest();
   case_fmt_hex_is_lower_case();