From a29e606b6f97c00d0b7e80cb0592bca54f92f2c9 Mon Sep 17 00:00:00 2001 From: Dirk Engling Date: Fri, 6 Nov 2026 21:17:54 +0100 Subject: Fix a theoretical bug where a short request packet yields an oob read --- src/ot_http.c | 6 +++--- tests/integration/test_keepalive.py | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/ot_http.c b/src/ot_http.c index fc49333..1b5281c 100644 --- a/src/ot_http.c +++ b/src/ot_http.c @@ -762,8 +762,8 @@ ssize_t http_handle_request(const int64_t sock, struct ot_workstruct *ws) { } } - /* This one implicitely tests strlen < 5, too -- remember, it is \n terminated */ - if (memcmp(read_ptr, "GET /", 5)) + /* The request need not be NUL terminated and can be shorter than the literal */ + if (ws->request_size < 5 || memcmp(read_ptr, "GET /", 5)) HTTPERROR_400; /* Skip leading '/' */ @@ -786,7 +786,7 @@ ssize_t http_handle_request(const int64_t sock, struct ot_workstruct *ws) { if ((*write_ptr == 'a') || (*write_ptr == '?')) http_handle_announce(sock, ws, read_ptr); #ifdef WANT_FULLSCRAPE - else if (!memcmp(write_ptr, "scrape HTTP/", 12)) + else if (ws->request + ws->request_size - write_ptr >= 12 && !memcmp(write_ptr, "scrape HTTP/", 12)) http_handle_fullscrape(sock, ws); #endif /* This is the hardcore match for scrape */ 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 -- cgit v1.3