diff --git a/darkhttpd.c b/darkhttpd.c index bcfb248..174246a 100644 --- a/darkhttpd.c +++ b/darkhttpd.c @@ -1778,13 +1778,17 @@ static void redirect(struct connection *conn, const char *format, ...) { */ static char *parse_field(const struct connection *conn, const char *field) { size_t bound1, bound2; - char *pos; - - /* find start */ - pos = strcasestr(conn->request, field); + char *search, *pos; + + /* Prepend '\n' so strcasestr only matches [field] at the start of a line, + * preventing substrings inside the request-target or another header's + * value from being mistaken for a real header. */ + xasprintf(&search, "\n%s", field); + pos = strcasestr(conn->request, search); + free(search); if (pos == NULL) return NULL; - assert(pos >= conn->request); + pos++; /* skip the leading '\n' */ bound1 = (size_t)(pos - conn->request) + strlen(field); /* find end */ diff --git a/devel/test_log_xff.py b/devel/test_log_xff.py index 9daad7b..687ac6a 100644 --- a/devel/test_log_xff.py +++ b/devel/test_log_xff.py @@ -90,6 +90,58 @@ def test_xff_ipv6(self): self.assertTrue(line.startswith(ipv6 + " "), f"Expected log to start with IPv6 {ipv6}. Log line: {line}") + def test_xff_smuggled_in_referer(self): + """ + A client without a real X-Forwarded-For header must not be able to + spoof its logged IP by smuggling the field name inside another + header's value (here: Referer). The log must show the real client + IP, not the smuggled one. + """ + spoof_ip = "99.99.99.99" + time_marker = random_str() + url = f"/xff_smuggle-{time_marker}" + + # No X-Forwarded-For header is sent; the substring lives inside + # the Referer value. Pre-fix parse_field used strcasestr() over + # the whole request and would match this. + self.get(url, req_hdrs={"Referer": f"X-Forwarded-For: {spoof_ip}"}) + + line = self._wait_and_check_log(url) + self.assertFalse(line.startswith(spoof_ip + " "), + f"Spoofed IP {spoof_ip} smuggled via Referer was logged as the " + f"client address. Log line: {line}") + self.assertTrue(line.startswith("127.0.0.1 "), + f"Expected log to start with real client IP 127.0.0.1. " + f"Log line: {line}") + + def test_xff_real_header_not_masked_by_prior_value_match(self): + """ + When the field name appears inside a prior header's value *and* a real + X-Forwarded-For header is also present, the real header must still be + found. A single-hit approach (find the first strcasestr match, check + pos[-1] != '\\n', stop) would reject that hit and return NULL, losing + the real header entirely. + """ + real_ip = "10.20.30.40" + spoof_ip = "99.99.99.99" + time_marker = random_str() + url = f"/xff_masked-{time_marker}" + + # Referer value contains "X-Forwarded-For: " as a substring. + # The real X-Forwarded-For header comes after. + self.get(url, req_hdrs={ + "Referer": f"http://evil.example/X-Forwarded-For: {spoof_ip}", + "X-Forwarded-For": real_ip, + }) + + line = self._wait_and_check_log(url) + self.assertTrue(line.startswith(real_ip + " "), + f"Expected log to start with real XFF IP {real_ip}. " + f"Log line: {line}") + self.assertFalse(line.startswith(spoof_ip + " "), + f"Spoofed IP {spoof_ip} (embedded in Referer value) was logged. " + f"Log line: {line}") + def test_xff_ipv6_list(self): """ Test a list containing IPv6 and IPv4.