Fix: livesync watermark used sizeof(ot_peer) which is a byte. Clamping on copy length now

2 files changed, 18 insertions, 1 deletions

diff --git a/src/ot_livesync.c b/src/ot_livesync.c
index 7d7208e..1381b3b 100644
--- a/src/ot_livesync.c
+++ b/src/ot_livesync.c
@@ -30,7 +30,7 @@ static uint16_t g_port  = LIVESYNC_PORT;
 #define LIVESYNC_INCOMING_BUFFSIZE        (256 * 256)
 
 #define LIVESYNC_OUTGOING_BUFFSIZE_PEERS  1480
-#define LIVESYNC_OUTGOING_WATERMARK_PEERS (sizeof(ot_peer) + sizeof(ot_hash))
+#define LIVESYNC_OUTGOING_WATERMARK_PEERS (sizeof(ot_hash) + OT_PEER_SIZE6)
 
 #define LIVESYNC_MAXDELAY                 15 /* seconds */
 
@@ -127,6 +127,10 @@ static void livesync_issue_peersync(sync_buffer *buf) {
   char   mycopy[LIVESYNC_OUTGOING_BUFFSIZE_PEERS];
   size_t fill = buf->fill;
 
+  /* The flush watermark keeps fill within the buffer */
+  if (fill > LIVESYNC_OUTGOING_BUFFSIZE_PEERS)
+    fill = LIVESYNC_OUTGOING_BUFFSIZE_PEERS;
+
   memcpy(mycopy, buf->data, fill);
   buf->fill             = sizeof(g_tracker_id) + sizeof(uint32_t);
   buf->next_packet_time = time_future_seconds(LIVESYNC_MAXDELAY);
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))