From 6202a32514a96277833ab642502f4425dc152b6a Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 09:39:17 +0200 Subject: [PATCH 01/24] F-5009: reject path traversal and absolute paths in TFTP RRQ/WRQ filenames The server copied the wire-supplied RRQ/WRQ filename verbatim and handed it straight to io.open() with no sanitization. Because TFTP is unauthenticated, an off-path sender could request "../../etc/passwd" (read) or "/etc/cron.d/evil" (write) and the raw traversal path reached an integrator-supplied, possibly-filesystem-backed open() unchanged. Add a component-aware wolftftp_filename_is_safe() in wolftftp_parse_request(): reject absolute paths (leading '/' or '\') and any ".." path component, while still allowing relative subdirectories and dots embedded in a name ("fw..bin"). A rejected request fails parsing as WOLFTFTP_ERR_PACKET, so the server replies with an error and never reaches io.open(). Document the (now-enforced) contract on the open callback in the header. --- src/test/unit/unit_tests_tftp.c | 54 +++++++++++++++++++++++++++++++++ src/tftp/wolftftp.c | 30 ++++++++++++++++++ src/tftp/wolftftp.h | 6 ++++ 3 files changed, 90 insertions(+) diff --git a/src/test/unit/unit_tests_tftp.c b/src/test/unit/unit_tests_tftp.c index e1207cd6..a4a6fb5c 100644 --- a/src/test/unit/unit_tests_tftp.c +++ b/src/test/unit/unit_tests_tftp.c @@ -330,6 +330,59 @@ START_TEST(test_tftp_parse_request_error_paths) } END_TEST +/* F-5009: an unauthenticated RRQ/WRQ filename must not be able to escape the + * integrator's namespace. parse_request rejects any '..' path component and any + * absolute path before the name can reach io.open. Benign names that merely + * contain dots (but no '..' component) and relative subdirectories still pass. */ +START_TEST(test_tftp_parse_request_rejects_path_traversal) +{ + uint8_t pkt[160]; + struct wolftftp_parsed_req req; + static const char *bad[] = { + "../../etc/passwd", /* leading traversal */ + "/etc/cron.d/evil", /* absolute (unix) */ + "\\windows\\sys", /* absolute (dos) */ + "..", /* whole name is a traversal */ + "fw/../../secret", /* embedded traversal */ + "images/..", /* trailing traversal component */ + "a/..\\b", /* backslash-separated traversal*/ + }; + static const char *good[] = { + "fw.bin", /* ordinary name */ + "images/fw.bin", /* relative subdir is fine */ + "fw..bin", /* dots, but not a '..' component */ + "v1.2..3.img", /* same */ + ".config", /* leading dot, not traversal */ + }; + size_t i, fnlen; + + for (i = 0; i < sizeof(bad) / sizeof(bad[0]); i++) { + memset(pkt, 0, sizeof(pkt)); + wolftftp_write_u16(pkt, WOLFTFTP_OP_RRQ); + fnlen = strlen(bad[i]); + memcpy(pkt + 2, bad[i], fnlen + 1U); + memcpy(pkt + 2 + fnlen + 1U, "octet", 6); + ck_assert_int_eq(wolftftp_parse_request(pkt, + (uint16_t)(2U + fnlen + 1U + 6U), &req), WOLFTFTP_ERR_PACKET); + /* Same rejection on the WRQ (arbitrary-write) path. */ + wolftftp_write_u16(pkt, WOLFTFTP_OP_WRQ); + ck_assert_int_eq(wolftftp_parse_request(pkt, + (uint16_t)(2U + fnlen + 1U + 6U), &req), WOLFTFTP_ERR_PACKET); + } + + for (i = 0; i < sizeof(good) / sizeof(good[0]); i++) { + memset(pkt, 0, sizeof(pkt)); + wolftftp_write_u16(pkt, WOLFTFTP_OP_RRQ); + fnlen = strlen(good[i]); + memcpy(pkt + 2, good[i], fnlen + 1U); + memcpy(pkt + 2 + fnlen + 1U, "octet", 6); + ck_assert_int_eq(wolftftp_parse_request(pkt, + (uint16_t)(2U + fnlen + 1U + 6U), &req), 0); + ck_assert_str_eq(req.filename, good[i]); + } +} +END_TEST + START_TEST(test_tftp_client_rrq_oack_and_data_success) { struct tftp_test_ctx ctx; @@ -2045,6 +2098,7 @@ static void add_tftp_tests(TCase *tc_proto) { tcase_add_test(tc_proto, test_tftp_helpers_and_builders); tcase_add_test(tc_proto, test_tftp_parse_request_error_paths); + tcase_add_test(tc_proto, test_tftp_parse_request_rejects_path_traversal); tcase_add_test(tc_proto, test_tftp_client_rrq_oack_and_data_success); tcase_add_test(tc_proto, test_tftp_client_fallback_duplicate_and_tid_errors); tcase_add_test(tc_proto, test_tftp_client_error_and_failure_paths); diff --git a/src/tftp/wolftftp.c b/src/tftp/wolftftp.c index ee78caeb..6ec3ef76 100644 --- a/src/tftp/wolftftp.c +++ b/src/tftp/wolftftp.c @@ -272,6 +272,34 @@ static int wolftftp_build_request(uint8_t *buf, uint16_t max_len, uint16_t opcod return 0; } +/* Reject filenames that could escape the integrator's namespace: any absolute + * path (leading '/' or '\') and any ".." path component. The library hands the + * name straight to io.open(), and TFTP is unauthenticated, so a naive + * filesystem-backed open() would otherwise be exposed to traversal. The check + * is component-aware: a ".." segment between separators (or as the whole name) + * is rejected, but dots inside a name ("fw..bin") are fine. */ +static int wolftftp_filename_is_safe(const char *name) +{ + const char *seg = name; + const char *c; + + if (name[0] == '\0') + return 0; + if (name[0] == '/' || name[0] == '\\') + return 0; + for (c = name; ; c++) { + if (*c == '/' || *c == '\\' || *c == '\0') { + size_t seglen = (size_t)(c - seg); + if (seglen == 2U && seg[0] == '.' && seg[1] == '.') + return 0; + if (*c == '\0') + break; + seg = c + 1; + } + } + return 1; +} + static int wolftftp_parse_request(const uint8_t *buf, uint16_t len, struct wolftftp_parsed_req *req) { @@ -294,6 +322,8 @@ static int wolftftp_parse_request(const uint8_t *buf, uint16_t len, return WOLFTFTP_ERR_PACKET; memcpy(req->filename, p, slen); req->filename[slen] = '\0'; + if (!wolftftp_filename_is_safe(req->filename)) + return WOLFTFTP_ERR_PACKET; p += slen + 1U; slen = wolftftp_strnlen_local(p, (size_t)(buf + len - (const uint8_t *)p)); if (slen == 0 || wolftftp_stricmp_local(p, "octet") != 0) diff --git a/src/tftp/wolftftp.h b/src/tftp/wolftftp.h index eba2e4d5..085efea7 100644 --- a/src/tftp/wolftftp.h +++ b/src/tftp/wolftftp.h @@ -143,6 +143,12 @@ struct wolftftp_negotiated { typedef int (*wolftftp_udp_send_cb)(void *arg, uint16_t local_port, const struct wolftftp_endpoint *remote, const uint8_t *buf, uint16_t len); +/* wolftftp_open_cb: + * `name` is the wire-supplied request filename. The server rejects absolute + * paths and any ".." path component before this callback runs, so `name` + * cannot escape upward from a relative root. It may still contain relative + * subdirectories ("a/b/fw.bin"); a filesystem-backed open() should resolve it + * against a confined root (chroot / fixed base dir) rather than the cwd. */ typedef int (*wolftftp_open_cb)(void *arg, const char *name, int is_write, uint32_t *size_hint, void **handle); /* wolftftp_read_cb: From 227c30ee4809006ca660d96fc245f601d9fd13dd Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 09:45:10 +0200 Subject: [PATCH 02/24] F-5481: put the ACK-triggering segment in the first SACK block tcp_rebuild_rx_sack() derived SACK blocks from the out-of-order cache and emitted them in descending-sequence order, so the first block was whichever island had the highest sequence rather than the segment that triggered the ACK. RFC 2018 sec.4 (2) requires the first SACK block to contain the segment that triggered the ACK (unless that segment advanced the cumulative ACK). With descending order the freshest island can be reported last and is the first to be dropped when the TCP option header lacks room for every block (e.g. four islands alongside a timestamp option, leaving room for only three blocks), delaying the peer's loss recovery. Pass the triggering segment's range into tcp_rebuild_rx_sack() from the store-OOO path and move the merged block containing it to rx_sack[0], keeping the remaining islands in descending order behind it. The consume/promote path advances the cumulative ACK, which is the RFC exemption, so it passes trig_len == 0 and keeps plain descending order. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_tcp_ack.c | 42 ++++++++++++++++++++++++++++++ src/wolfip.c | 33 +++++++++++++++++++---- 3 files changed, 71 insertions(+), 5 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index e8b57e5b..53bd523f 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -644,6 +644,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_tcp_merge_sack_blocks_wrap_order); tcase_add_test(tc_utils, test_tcp_recv_tracks_holes_and_sack_blocks); tcase_add_test(tc_utils, test_tcp_rebuild_rx_sack_right_edge_wraps); + tcase_add_test(tc_utils, test_tcp_rebuild_rx_sack_triggering_block_first); tcase_add_test(tc_utils, test_tcp_consume_ooo_wrap_trim_and_promote); tcase_add_test(tc_utils, test_tcp_consume_ooo_wrap_drop_fully_acked); tcase_add_test(tc_utils, test_tcp_ack_sack_early_retransmit_before_three_dupack); diff --git a/src/test/unit/unit_tests_tcp_ack.c b/src/test/unit/unit_tests_tcp_ack.c index 954dc1d0..5b674a6d 100644 --- a/src/test/unit/unit_tests_tcp_ack.c +++ b/src/test/unit/unit_tests_tcp_ack.c @@ -4331,6 +4331,48 @@ START_TEST(test_tcp_rebuild_rx_sack_right_edge_wraps) } END_TEST +/* F-5481: RFC 2018 sec.4 (2) - the first SACK block MUST describe the segment + * that triggered the ACK. Store three disjoint islands so descending-sequence + * order would put the most recently received (lowest) island last; the rebuild + * must instead promote the triggering segment's block to rx_sack[0]. */ +START_TEST(test_tcp_rebuild_rx_sack_triggering_block_first) +{ + struct wolfIP s; + struct tsocket *ts; + uint8_t payload[16] = {0}; + + wolfIP_init(&s); + ts = &s.tcpsockets[0]; + memset(ts, 0, sizeof(*ts)); + ts->proto = WI_IPPROTO_TCP; + ts->S = &s; + ts->sock.tcp.state = TCP_ESTABLISHED; + + ck_assert_int_eq(tcp_store_ooo_segment(ts, payload, 300, 10), 0); + ck_assert_int_eq(tcp_store_ooo_segment(ts, payload, 400, 10), 0); + /* This is the freshest segment and the lowest sequence: under a plain + * descending sort it would be reported last and dropped first on + * option-space truncation. */ + ck_assert_int_eq(tcp_store_ooo_segment(ts, payload, 200, 10), 0); + + ck_assert_uint_eq(ts->sock.tcp.rx_sack_count, 3); + ck_assert_uint_eq(ts->sock.tcp.rx_sack[0].left, 200); + ck_assert_uint_eq(ts->sock.tcp.rx_sack[0].right, 210); + /* Remaining islands keep descending order behind the required first block. */ + ck_assert_uint_eq(ts->sock.tcp.rx_sack[1].left, 400); + ck_assert_uint_eq(ts->sock.tcp.rx_sack[2].left, 300); + + /* Closing holes advances the cumulative ACK (the RFC exemption), so the + * consume path reports remaining islands in plain descending order. */ + ts->sock.tcp.ack = 200; + queue_init(&ts->sock.tcp.rxbuf, ts->rxmem, RXBUF_SIZE, 200); + tcp_consume_ooo(ts); + ck_assert_uint_eq(ts->sock.tcp.rx_sack_count, 2); + ck_assert_uint_eq(ts->sock.tcp.rx_sack[0].left, 400); + ck_assert_uint_eq(ts->sock.tcp.rx_sack[1].left, 300); +} +END_TEST + START_TEST(test_tcp_consume_ooo_wrap_trim_and_promote) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 9d303ff2..2cc303b9 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -2910,7 +2910,8 @@ static uint8_t tcp_merge_sack_blocks(struct tcp_sack_block *blocks, uint8_t coun return (uint8_t)(out + 1); } -static void tcp_rebuild_rx_sack(struct tsocket *t) +static void tcp_rebuild_rx_sack(struct tsocket *t, uint32_t trig_seq, + uint32_t trig_len) { struct tcp_sack_block blocks[TCP_OOO_MAX_SEGS]; uint8_t i, count = 0; @@ -2936,6 +2937,26 @@ static void tcp_rebuild_rx_sack(struct tsocket *t) count--; t->sock.tcp.rx_sack[t->sock.tcp.rx_sack_count++] = blocks[count]; } + + /* RFC 2018 sec.4 (2): the first SACK block MUST contain the segment that + * triggered this ACK (unless that segment advanced the cumulative ACK, in + * which case the caller passes trig_len == 0). The loop above leaves blocks + * in descending-sequence order, so the triggering island can be anywhere; + * move the merged block holding it to the front. This also guarantees it + * survives truncation when the TCP option lacks room for every block. */ + if (trig_len != 0U) { + for (i = 0; i < t->sock.tcp.rx_sack_count; i++) { + struct tcp_sack_block *b = &t->sock.tcp.rx_sack[i]; + if (tcp_seq_leq(b->left, trig_seq) && tcp_seq_lt(trig_seq, b->right)) { + struct tcp_sack_block first = t->sock.tcp.rx_sack[i]; + uint8_t j; + for (j = i; j > 0; j--) + t->sock.tcp.rx_sack[j] = t->sock.tcp.rx_sack[j - 1]; + t->sock.tcp.rx_sack[0] = first; + break; + } + } + } } static int tcp_store_ooo_segment(struct tsocket *t, const uint8_t *data, @@ -2964,7 +2985,7 @@ static int tcp_store_ooo_segment(struct tsocket *t, const uint8_t *data, /* Duplicate range: keep newest bytes and avoid consuming another slot. */ if (t->sock.tcp.ooo[i].seq == seq && t->sock.tcp.ooo[i].len == len) { memcpy(t->sock.tcp.ooo[i].data, data, len); - tcp_rebuild_rx_sack(t); + tcp_rebuild_rx_sack(t, seq, len); return 0; } } @@ -2975,7 +2996,7 @@ static int tcp_store_ooo_segment(struct tsocket *t, const uint8_t *data, t->sock.tcp.ooo[slot].seq = seq; t->sock.tcp.ooo[slot].len = len; memcpy(t->sock.tcp.ooo[slot].data, data, len); - tcp_rebuild_rx_sack(t); + tcp_rebuild_rx_sack(t, seq, len); return 0; } @@ -3039,8 +3060,10 @@ static void tcp_consume_ooo(struct tsocket *t) } } /* Rebuild advertised SACK blocks from whatever OOO cache remains after - * promotion. If all holes closed, this naturally drops SACK reporting. */ - tcp_rebuild_rx_sack(t); + * promotion. If all holes closed, this naturally drops SACK reporting. + * Promotion advances the cumulative ACK, so RFC 2018's "first block holds + * the triggering segment" rule does not apply here (trig_len == 0). */ + tcp_rebuild_rx_sack(t, 0, 0); } static uint8_t tcp_build_ack_options(struct tsocket *t, uint8_t *opt, uint8_t max_len) From 1ed5dc9d32a439562d2c8ca7a845510d27ff8157 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 09:54:23 +0200 Subject: [PATCH 03/24] F-4946: guard DNS lookups against clobbering in-flight query state nslookup() and wolfIP_dns_ptr_lookup() overwrote the shared dns_lookup_cb, dns_ptr_cb and dns_query_type fields before calling dns_send_query(), whose "dns_id != 0" busy-guard only fires afterwards. A second DNS API call made while a query was in flight was rejected with -16, but by then it had already replaced the in-flight query's callback/type state. When the original query's response arrived (its id still matched dns_id, since the second call never changed it) dns_callback() invoked the wrong handler with that response's rdata (A->A handler hijack), or the PTR path NULLed the A callback and flipped the query type, stalling the resolver until the retry timer gave up. Move the busy-guard ahead of any state mutation in both entry points: when dns_id != 0 return -16 before touching the shared fields. The guard inside dns_send_query() is retained as defence in depth. --- src/test/unit/unit.c | 1 + src/test/unit/unit_shared.c | 7 +++ src/test/unit/unit_tests_dns_dhcp.c | 86 +++++++++++++++++++++++++++++ src/wolfip.c | 8 +++ 4 files changed, 102 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 53bd523f..2e7aaa25 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -434,6 +434,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_udp_try_recv_full_fifo_drop_does_not_set_readable_or_send_icmp); tcase_add_test(tc_utils, test_dns_callback_bad_flags); tcase_add_test(tc_utils, test_dns_callback_truncated_response_aborts_query); + tcase_add_test(tc_utils, test_dns_inflight_query_state_not_clobbered_by_second_call); tcase_add_test(tc_utils, test_regression_dns_callback_high_bit_octet_ip_no_ub); tcase_add_test(tc_utils, test_dns_callback_bad_name); tcase_add_test(tc_utils, test_dns_callback_short_header_ignored); diff --git a/src/test/unit/unit_shared.c b/src/test/unit/unit_shared.c index 21ddd742..2fcda51c 100644 --- a/src/test/unit/unit_shared.c +++ b/src/test/unit/unit_shared.c @@ -139,6 +139,7 @@ static uint16_t socket_cb_last_events; static int timer_cb_calls; static uint32_t dns_lookup_ip; static int dns_lookup_calls; +static int dns_lookup_evil_calls; struct tcp_seg_buf { struct wolfIP_tcp_seg seg; @@ -302,6 +303,12 @@ static void test_dns_lookup_cb(uint32_t ip) dns_lookup_calls++; } +static void test_dns_lookup_evil_cb(uint32_t ip) +{ + (void)ip; + dns_lookup_evil_calls++; +} + static void test_dns_ptr_cb(const char *name) { (void)name; diff --git a/src/test/unit/unit_tests_dns_dhcp.c b/src/test/unit/unit_tests_dns_dhcp.c index 737fcf34..cb9e79db 100644 --- a/src/test/unit/unit_tests_dns_dhcp.c +++ b/src/test/unit/unit_tests_dns_dhcp.c @@ -5611,6 +5611,92 @@ START_TEST(test_dns_callback_truncated_response_aborts_query) } END_TEST +/* F-4946: a second DNS API call while a query is in-flight must not touch the + * shared callback/type state. Before the fix, nslookup/wolfIP_dns_ptr_lookup + * overwrote dns_lookup_cb/dns_ptr_cb/dns_query_type before dns_send_query's + * busy-guard ran, so the in-flight query's response invoked the wrong handler + * (A->A hijack) or stalled the resolver (A->PTR flip). */ +START_TEST(test_dns_inflight_query_state_not_clobbered_by_second_call) +{ + struct wolfIP s; + uint16_t id1 = 0, id2 = 0, id3 = 0; + uint8_t response[128]; + int pos; + struct dns_header *hdr = (struct dns_header *)response; + struct dns_question *q; + struct dns_rr *rr; + const uint8_t ip_bytes[4] = {0x01, 0x02, 0x03, 0x04}; + + wolfIP_init(&s); + mock_link_init(&s); + s.dns_server = 0x08080808U; + s.last_tick = 100U; + + dns_lookup_calls = 0; + dns_lookup_ip = 0; + dns_lookup_evil_calls = 0; + + /* First lookup goes out: dns_id armed, target callback recorded. */ + ck_assert_int_eq(nslookup(&s, "target.com", &id1, test_dns_lookup_cb), 0); + ck_assert_uint_ne(id1, 0U); + ck_assert_uint_eq(s.dns_id, id1); + ck_assert_ptr_eq(s.dns_lookup_cb, test_dns_lookup_cb); + ck_assert_int_eq(s.dns_query_type, DNS_QUERY_TYPE_A); + + /* A->A arm: a second nslookup while in-flight must be rejected as busy + * without replacing the recorded callback. */ + ck_assert_int_eq(nslookup(&s, "evil.com", &id2, test_dns_lookup_evil_cb), -16); + ck_assert_uint_eq(s.dns_id, id1); + ck_assert_ptr_eq(s.dns_lookup_cb, test_dns_lookup_cb); + ck_assert_ptr_eq(s.dns_ptr_cb, NULL); + ck_assert_int_eq(s.dns_query_type, DNS_QUERY_TYPE_A); + + /* A->PTR arm: a PTR lookup while in-flight must not NULL the A callback or + * flip the query type. */ + ck_assert_int_eq(wolfIP_dns_ptr_lookup(&s, 0x08080808U, &id3, + test_dns_ptr_cb), -16); + ck_assert_uint_eq(s.dns_id, id1); + ck_assert_ptr_eq(s.dns_lookup_cb, test_dns_lookup_cb); + ck_assert_ptr_eq(s.dns_ptr_cb, NULL); + ck_assert_int_eq(s.dns_query_type, DNS_QUERY_TYPE_A); + + /* The response to the original (still in-flight) query must reach the + * original handler, never the rejected second call's handler. */ + memset(response, 0, sizeof(response)); + hdr->id = ee16(id1); + hdr->flags = ee16(0x8100); + hdr->qdcount = ee16(1); + hdr->ancount = ee16(1); + pos = sizeof(struct dns_header); + response[pos++] = 6; memcpy(&response[pos], "target", 6); pos += 6; + response[pos++] = 3; memcpy(&response[pos], "com", 3); pos += 3; + response[pos++] = 0; + q = (struct dns_question *)(response + pos); + q->qtype = ee16(DNS_A); + q->qclass = ee16(DNS_CLASS_IN); + pos += sizeof(struct dns_question); + response[pos++] = 0xC0; + response[pos++] = (uint8_t)sizeof(struct dns_header); + rr = (struct dns_rr *)(response + pos); + rr->type = ee16(DNS_A); + rr->class = ee16(DNS_CLASS_IN); + rr->ttl = ee32(60); + rr->rdlength = ee16(4); + pos += sizeof(struct dns_rr); + memcpy(&response[pos], ip_bytes, sizeof(ip_bytes)); + pos += sizeof(ip_bytes); + + enqueue_udp_rx(&s.udpsockets[SOCKET_UNMARK(s.dns_udp_sd)], response, + (uint16_t)pos, DNS_PORT); + dns_callback(s.dns_udp_sd, CB_EVENT_READABLE, &s); + + ck_assert_int_eq(dns_lookup_calls, 1); + ck_assert_uint_eq(dns_lookup_ip, 0x01020304U); + ck_assert_int_eq(dns_lookup_evil_calls, 0); + ck_assert_uint_eq(s.dns_id, 0U); +} +END_TEST + START_TEST(test_regression_dns_callback_high_bit_octet_ip_no_ub) { /* The dns_callback() A-record reassembly used to compute diff --git a/src/wolfip.c b/src/wolfip.c index 2cc303b9..551f3d4c 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -9419,6 +9419,11 @@ int nslookup(struct wolfIP *s, const char *dname, uint16_t *id, void (*lookup_cb { if (!s || !dname || !id || !lookup_cb) return -22; + /* Reject before touching shared callback/type state: dns_send_query's + * busy-guard runs too late to stop an in-flight query's callback from + * being clobbered by this (rejected) call. */ + if (s->dns_id != 0) + return -16; s->dns_lookup_cb = lookup_cb; s->dns_ptr_cb = NULL; s->dns_query_type = DNS_QUERY_TYPE_A; @@ -9432,6 +9437,9 @@ int wolfIP_dns_ptr_lookup(struct wolfIP *s, uint32_t ip, uint16_t *id, void (*lo return -22; if (dns_format_ptr_name(ptr_name, sizeof(ptr_name), ip) < 0) return -22; + /* Reject before touching shared callback/type state (see nslookup). */ + if (s->dns_id != 0) + return -16; s->dns_ptr_cb = lookup_cb; s->dns_lookup_cb = NULL; s->dns_ptr_name[0] = DNS_NAME_TERMINATOR; From 62bdc9f41a5f90b258e39b9cbfb7f9a9ea243cd1 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 10:11:37 +0200 Subject: [PATCH 04/24] F-5070: enforce raw socket bound_local_ip and if_idx on receive raw_try_recv() delivered every protocol-matching IPv4 packet to a bound raw socket without consulting r->bound_local_ip or r->if_idx (the arriving interface was explicitly discarded with (void)if_idx). wolfIP_sock_bind() accepts a local IP and interface for raw sockets and records them, so a socket bound to one local address still captured traffic for every other destination, and a socket bound to one interface still saw traffic from all of them. With forwarding enabled raw_try_recv() runs on the IP input path, so a bound socket even saw transit traffic for unrelated destinations. The TCP/UDP receive paths already gate on bound_local_ip; raw sockets were the outlier. Add the same two guards inside the socket-match loop: skip when bound_local_ip is set and differs from the packet destination, and skip when if_idx is set and differs from the arriving interface (IPADDR_ANY / 0 keep "match any"). Unbound catch-all sockets are unaffected. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_proto.c | 73 ++++++++++++++++++++++++++++++++ src/wolfip.c | 9 +++- 3 files changed, 82 insertions(+), 1 deletion(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 2e7aaa25..28838c08 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -775,6 +775,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_icmp_try_recv_mismatch_remote_ip); tcase_add_test(tc_proto, test_icmp_try_recv_full_fifo_does_not_signal_readable); tcase_add_test(tc_proto, test_raw_socket_recv_captures_ip_header); + tcase_add_test(tc_proto, test_raw_socket_recv_honors_bound_local_ip_and_if); tcase_add_test(tc_proto, test_raw_socket_send_hdrincl_respected); tcase_add_test(tc_proto, test_raw_socket_send_builds_ip_header); tcase_add_test(tc_proto, test_regression_raw_socket_send_ip_id_network_byte_order); diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index 15a8942e..7f94d4fe 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -6540,6 +6540,79 @@ START_TEST(test_raw_socket_recv_captures_ip_header) } END_TEST +/* F-5070: raw_try_recv must honour a raw socket's bound_local_ip and if_idx, + * mirroring the TCP/UDP bind contract. A socket bound to one local IP/interface + * must not receive protocol-matching traffic for other destinations or arriving + * on other interfaces. The frame is rebuilt before every injection because the + * unit build enables forwarding, which rewrites the buffer in place. */ +#define RAW_BIND_FRAME_LEN (ETH_HEADER_LEN + IP_HEADER_LEN + 8) +static void raw_bind_build_frame(struct wolfIP *s, struct wolfIP_ip_packet *frame, + size_t bufsz, uint32_t dst) +{ + static const uint8_t payload[8] = {1, 2, 3, 4, 5, 6, 7, 8}; + memset(frame, 0, bufsz); + memcpy(frame->eth.dst, s->ll_dev[TEST_PRIMARY_IF].mac, 6); + memcpy(frame->eth.src, "\xaa\xbb\xcc\xdd\xee\xff", 6); + frame->eth.type = ee16(ETH_TYPE_IP); + frame->ver_ihl = 0x45; + frame->ttl = 32; + frame->proto = WI_IPPROTO_UDP; + frame->len = ee16(IP_HEADER_LEN + (uint16_t)sizeof(payload)); + frame->src = ee32(0x0A000002U); + frame->dst = ee32(dst); + memcpy(frame->data, payload, sizeof(payload)); + iphdr_set_checksum(frame); +} + +START_TEST(test_raw_socket_recv_honors_bound_local_ip_and_if) +{ + struct wolfIP s; + int sd; + struct rawsocket *rs; + uint8_t frame_buf[sizeof(struct wolfIP_ip_packet) + 8]; + struct wolfIP_ip_packet *frame = (struct wolfIP_ip_packet *)frame_buf; + uint8_t rxbuf[64]; + const int delivered = IP_HEADER_LEN + 8; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_RAW, WI_IPPROTO_UDP); + ck_assert_int_ge(sd, 0); + rs = &s.rawsockets[SOCKET_UNMARK(sd)]; + + /* Bound to 10.0.0.1: a packet destined elsewhere must be filtered out. */ + rs->bound_local_ip = 0x0A000001U; + rs->if_idx = 0; + raw_bind_build_frame(&s, frame, sizeof(frame_buf), 0x0A0000FEU); + wolfIP_recv_ex(&s, TEST_PRIMARY_IF, frame, RAW_BIND_FRAME_LEN); + ck_assert_int_eq(wolfIP_sock_recvfrom(&s, sd, rxbuf, sizeof(rxbuf), 0, + NULL, 0), -WOLFIP_EAGAIN); + + /* Same socket, packet destined to the bound IP: delivered. */ + raw_bind_build_frame(&s, frame, sizeof(frame_buf), 0x0A000001U); + wolfIP_recv_ex(&s, TEST_PRIMARY_IF, frame, RAW_BIND_FRAME_LEN); + ck_assert_int_eq(wolfIP_sock_recvfrom(&s, sd, rxbuf, sizeof(rxbuf), 0, + NULL, 0), delivered); + + /* Catch-all destination but bound to a different interface: filtered. */ + rs->bound_local_ip = IPADDR_ANY; + rs->if_idx = (uint8_t)(TEST_PRIMARY_IF + 1U); + raw_bind_build_frame(&s, frame, sizeof(frame_buf), 0x0A000001U); + wolfIP_recv_ex(&s, TEST_PRIMARY_IF, frame, RAW_BIND_FRAME_LEN); + ck_assert_int_eq(wolfIP_sock_recvfrom(&s, sd, rxbuf, sizeof(rxbuf), 0, + NULL, 0), -WOLFIP_EAGAIN); + + /* if_idx == 0 means "any interface": delivered on the arriving one. */ + rs->if_idx = 0; + raw_bind_build_frame(&s, frame, sizeof(frame_buf), 0x0A000001U); + wolfIP_recv_ex(&s, TEST_PRIMARY_IF, frame, RAW_BIND_FRAME_LEN); + ck_assert_int_eq(wolfIP_sock_recvfrom(&s, sd, rxbuf, sizeof(rxbuf), 0, + NULL, 0), delivered); +} +END_TEST + START_TEST(test_raw_socket_send_hdrincl_respected) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 551f3d4c..7747c024 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -2604,13 +2604,20 @@ static void raw_try_recv(struct wolfIP *s, unsigned int if_idx, struct wolfIP_ip if (ip_len > payload_len) return; payload_len = ip_len; - (void)if_idx; for (int i = 0; i < WOLFIP_MAX_RAWSOCKETS; i++) { struct rawsocket *r = &s->rawsockets[i]; if (!r->used) continue; if (r->protocol != 0 && r->protocol != ip->proto) continue; + /* Honour the bind contract, mirroring the TCP/UDP receive paths: a + * socket bound to a specific local IP or interface must not capture + * traffic for other destinations or arriving on other interfaces. + * bound_local_ip == IPADDR_ANY / if_idx == 0 mean "any". */ + if (r->bound_local_ip != IPADDR_ANY && r->bound_local_ip != ee32(ip->dst)) + continue; + if (r->if_idx != 0 && r->if_idx != (uint8_t)if_idx) + continue; if (fifo_push(&r->rxbuf, (void *)packet, payload_len) == 0) { r->last_pkt_ttl = ip->ttl; r->events |= CB_EVENT_READABLE; From ae1f904e45eb34be6577a402f74cac2f58e7a4d4 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 11:01:29 +0200 Subject: [PATCH 05/24] F-5696: deregister httpd callback on the lingering socket in fail_close http_recv_cb's fail_close path freed hc->ssl, called wolfIP_sock_close(sd) and zeroed hc->client_sd, but never deregistered the socket callback. For an ESTABLISHED connection wolfIP_sock_close only begins the active close: it moves the socket to FIN_WAIT_1, returns -EAGAIN, and leaves ts->callback / ts->callback_arg pointing at this http_client slot. With client_sd zeroed the slot looks free, so the next accept reuses it for a new connection (fresh sd, fresh ssl) and re-registers the callback with the same &clients[i] arg. The half-closed socket still accepts data in FIN_WAIT_1, so a late segment on it makes wolfIP_poll fire the stale callback against the new connection's state - freeing its live TLS context and zeroing its client_sd, a use-after-free / repeatable denial of service against concurrent HTTPS clients. Deregister the callback on the lingering socket (wolfIP_register_callback with NULL cb/arg) before zeroing client_sd, so a segment arriving on the half-closed socket can no longer dispatch into a reused slot. --- src/http/httpd.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/http/httpd.c b/src/http/httpd.c index 41e1cb55..bd1aabb0 100644 --- a/src/http/httpd.c +++ b/src/http/httpd.c @@ -429,6 +429,13 @@ static void http_recv_cb(int sd, uint16_t event, void *arg) { hc->ssl = NULL; } wolfIP_sock_close(hc->httpd->ipstack, sd); + /* wolfIP_sock_close on an ESTABLISHED socket only starts the active close + * (FIN_WAIT_1) and returns -EAGAIN; the socket lingers, still carrying this + * callback and its arg. Once we zero client_sd the slot is reused by the + * next accept, so the lingering socket's callback_arg would dangle onto a + * different live connection's state. Deregister it here so a late segment + * on the half-closed socket can no longer fire http_recv_cb. */ + wolfIP_register_callback(hc->httpd->ipstack, sd, NULL, NULL); hc->client_sd = 0; } From ebd710997226f504baafcc74b33114883dd19302 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 11:47:57 +0200 Subject: [PATCH 06/24] F-4501: drop filter-blocked packet-socket TX frames instead of resending The WOLFIP_PACKET_SOCKETS TX loop in wolfIP_poll walked the queue with fifo_next() when the SENDING filter blocked a frame, but fifo_pop() only ever removes the tail (oldest) descriptor. With [A,B] queued and A blocked, the loop advanced to B, sent it, then popped A; the next fifo_peek() returned B again, so B was transmitted a second time and the SENDING filter re-fired for it. A was silently dropped and B sent twice. fifo has no way to remove an arbitrary descriptor, so the fifo_next()/fifo_pop() mix cannot be made correct. Drop the filter-blocked frame from the head with fifo_pop() and re-peek, matching firewall-drop semantics and sending each accepted frame exactly once. The sibling TX loops (TCP/UDP/ICMP/raw) already break on a filter block and were not touched. --- src/test/unit/unit.c | 3 + src/test/unit/unit_tests_proto.c | 96 ++++++++++++++++++++++++++++++++ src/wolfip.c | 7 ++- 3 files changed, 105 insertions(+), 1 deletion(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 28838c08..251ccca5 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -788,6 +788,9 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_getsockopt_unsupported_option_returns_einval); tcase_add_test(tc_proto, test_packet_socket_recv_frame); tcase_add_test(tc_proto, test_packet_socket_send_frame); +#if WOLFIP_PACKET_SOCKETS + tcase_add_test(tc_proto, test_packet_socket_tx_filter_block_does_not_resend); +#endif tcase_add_test(tc_proto, test_packet_socket_sendto_wrong_family_returns_einval); tcase_add_test(tc_proto, test_packet_socket_setsockopt_rejected); tcase_add_test(tc_proto, test_packet_socket_getsockopt_rejected); diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index 7f94d4fe..f009558c 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -7028,6 +7028,102 @@ START_TEST(test_packet_socket_send_frame) } END_TEST +#if WOLFIP_PACKET_SOCKETS +/* F-4501: a SENDING-filter block must not desync the TX walk from fifo_pop(). + * Frame A (PKT_A_LEN) is blocked; frame B (PKT_B_LEN) is accepted. The filter + * counts how many times each length reaches the SENDING hook: the bug walked + * with fifo_next() but removed the tail with fifo_pop(), re-peeking and + * re-sending B (B seen twice). The fix drops the blocked frame and sends B + * exactly once. */ +#define PKT_F4501_A_LEN ((uint32_t)(ETH_HEADER_LEN + 8)) +#define PKT_F4501_B_LEN ((uint32_t)(ETH_HEADER_LEN + 16)) +static int pkt_f4501_block_a_calls; +static int pkt_f4501_b_send_calls; +static int pkt_f4501_filter_cb(void *arg, const struct wolfIP_filter_event *event) +{ + (void)arg; + if (event && event->reason == WOLFIP_FILT_SENDING) { + if (event->length == PKT_F4501_A_LEN) { + pkt_f4501_block_a_calls++; + return 1; /* block frame A */ + } + if (event->length == PKT_F4501_B_LEN) + pkt_f4501_b_send_calls++; + } + return 0; +} + +START_TEST(test_packet_socket_tx_filter_block_does_not_resend) +{ + struct wolfIP s; + int sd; + struct wolfIP_sockaddr_ll sll; + struct wolfIP_sockaddr_ll bind_sll; + uint8_t frame_a[PKT_F4501_A_LEN]; + uint8_t frame_b[PKT_F4501_B_LEN]; + struct wolfIP_eth_frame *eth_a = (struct wolfIP_eth_frame *)frame_a; + struct wolfIP_eth_frame *eth_b = (struct wolfIP_eth_frame *)frame_b; + struct packetsocket *ps; + + wolfIP_init(&s); + mock_link_init(&s); + + sd = wolfIP_sock_socket(&s, AF_PACKET, IPSTACK_SOCK_RAW, ee16(ETH_TYPE_IP)); + ck_assert_int_ge(sd, 0); + + memset(&bind_sll, 0, sizeof(bind_sll)); + bind_sll.sll_family = AF_PACKET; + bind_sll.sll_protocol = ee16(ETH_TYPE_IP); + bind_sll.sll_ifindex = TEST_PRIMARY_IF; + bind_sll.sll_halen = 6; + memset(bind_sll.sll_addr, 0xFF, 6); + ck_assert_int_eq(wolfIP_sock_bind(&s, sd, + (struct wolfIP_sockaddr *)&bind_sll, sizeof(bind_sll)), 0); + + memset(&sll, 0, sizeof(sll)); + sll.sll_family = AF_PACKET; + sll.sll_protocol = ee16(ETH_TYPE_IP); + sll.sll_ifindex = TEST_PRIMARY_IF; + sll.sll_halen = 6; + memset(sll.sll_addr, 0xFF, 6); + + /* Queue [A, B]; A is shorter so the filter can tell them apart. */ + memset(frame_a, 0, sizeof(frame_a)); + memcpy(eth_a->dst, "\xff\xff\xff\xff\xff\xff", 6); + memcpy(eth_a->src, "\x00\x00\x00\x00\x00\x01", 6); + eth_a->type = ee16(ETH_TYPE_IP); + memset(eth_a->data, 0xAA, sizeof(frame_a) - ETH_HEADER_LEN); + ck_assert_int_eq(wolfIP_sock_sendto(&s, sd, frame_a, sizeof(frame_a), 0, + (struct wolfIP_sockaddr *)&sll, sizeof(sll)), (int)sizeof(frame_a)); + + memset(frame_b, 0, sizeof(frame_b)); + memcpy(eth_b->dst, "\xff\xff\xff\xff\xff\xff", 6); + memcpy(eth_b->src, "\x00\x00\x00\x00\x00\x02", 6); + eth_b->type = ee16(ETH_TYPE_IP); + memset(eth_b->data, 0xBB, sizeof(frame_b) - ETH_HEADER_LEN); + ck_assert_int_eq(wolfIP_sock_sendto(&s, sd, frame_b, sizeof(frame_b), 0, + (struct wolfIP_sockaddr *)&sll, sizeof(sll)), (int)sizeof(frame_b)); + + pkt_f4501_block_a_calls = 0; + pkt_f4501_b_send_calls = 0; + wolfIP_filter_set_callback(pkt_f4501_filter_cb, NULL); + wolfIP_filter_set_mask(~0U); + + wolfIP_poll(&s, 1000); + + wolfIP_filter_set_callback(NULL, NULL); + wolfIP_filter_set_mask(0); + + /* A was filtered (and dropped), B was sent exactly once - not twice. */ + ck_assert_int_ge(pkt_f4501_block_a_calls, 1); + ck_assert_int_eq(pkt_f4501_b_send_calls, 1); + /* Both descriptors are gone: the blocked one is dropped, not left behind. */ + ps = &s.packetsockets[SOCKET_UNMARK(sd)]; + ck_assert_ptr_eq(fifo_peek(&ps->txbuf), NULL); +} +END_TEST +#endif /* WOLFIP_PACKET_SOCKETS */ + START_TEST(test_packet_socket_sendto_wrong_family_returns_einval) { #if WOLFIP_PACKET_SOCKETS diff --git a/src/wolfip.c b/src/wolfip.c index 7747c024..06c59df5 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -9926,7 +9926,12 @@ int wolfIP_poll(struct wolfIP *s, uint64_t now) tx_if = 0; if (wolfIP_filter_notify_eth(WOLFIP_FILT_SENDING, s, tx_if, (struct wolfIP_eth_frame *)frame, desc->len) != 0) { - desc = fifo_next(&p->txbuf, desc); + /* The filter vetoed this frame. fifo_pop() only removes the + * tail, so walking forward with fifo_next() here would later + * pop the wrong descriptor and re-send an accepted frame. Drop + * the blocked frame from the head and re-evaluate. */ + fifo_pop(&p->txbuf); + desc = fifo_peek(&p->txbuf); continue; } wolfIP_ll_send_frame(s, tx_if, frame, desc->len); From 7b9e310e7e5cc8dc93173788d76e2978d17c082d Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 11:59:10 +0200 Subject: [PATCH 07/24] F-5011: deliver 802.1Q-tagged frames to parent-bound AF_PACKET sockets wolfIP_recv_on() rebased if_idx from the physical parent to the matched VLAN sub-interface and stripped the tag in place before calling packet_try_recv(). Since packet_try_recv() matches sockets by if_idx, an AF_PACKET socket bound to the parent never matched and received zero tagged frames - blinding a parent-bound sniffer or IDS to all VLAN traffic - while wildcard listeners only saw the tag-stripped frame stamped with the sub-interface index, losing the VLAN membership information. Tap the physical interface with the original, still-tagged frame (parent if_idx) before the demux/strip, so parent-bound and wildcard sockets observe VLAN traffic in wire form. The existing post-demux delivery now runs in "exact interface" mode (new match_wildcard arg to packet_try_recv) so sockets bound explicitly to the sub-interface still get the tag-stripped copy while wildcard sockets are not delivered the same frame a second time. Non-VLAN and non-packet-socket paths are unchanged. --- src/test/unit/unit.c | 4 ++ src/test/unit/unit_tests_vlan.c | 119 ++++++++++++++++++++++++++++++++ src/wolfip.c | 36 ++++++++-- 3 files changed, 153 insertions(+), 6 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 251ccca5..a858f50a 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -1529,6 +1529,10 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_vlan_rx_dei_bit_accepted); tcase_add_test(tc_proto, test_vlan_rx_tagged_arp_processed); tcase_add_test(tc_proto, test_vlan_mtu_inherited); +#if WOLFIP_PACKET_SOCKETS + tcase_add_test(tc_proto, test_vlan_packet_socket_parent_and_sub_delivery); + tcase_add_test(tc_proto, test_vlan_packet_socket_wildcard_gets_tagged_once); +#endif #endif /* WOLFIP_VLAN */ suite_add_tcase(s, tc_core); diff --git a/src/test/unit/unit_tests_vlan.c b/src/test/unit/unit_tests_vlan.c index 1bed48b4..f9bf6cf4 100644 --- a/src/test/unit/unit_tests_vlan.c +++ b/src/test/unit/unit_tests_vlan.c @@ -948,6 +948,125 @@ START_TEST(test_vlan_rx_untagged_on_physical_ok) } END_TEST +#if WOLFIP_PACKET_SOCKETS +/* F-5011: a tagged frame is rebased onto the sub-interface before + * packet_try_recv ran, so an AF_PACKET socket bound to the parent saw nothing. + * After the fix the parent (and wildcard) listeners observe the original tagged + * frame, while a socket bound to the sub-interface still gets the tag-stripped + * copy. */ +START_TEST(test_vlan_packet_socket_parent_and_sub_delivery) +{ + struct wolfIP s; + struct wolfIP_ll_dev *phys; + unsigned int sub_idx = 0xFFFFFFFFu; + int sd_parent, sd_sub; + uint8_t plain[ETH_HEADER_LEN + IP_HEADER_LEN + ICMP_HEADER_LEN]; + uint8_t tagged[ETH_HEADER_LEN + IP_HEADER_LEN + ICMP_HEADER_LEN + 4]; + uint32_t plain_len, tagged_len; + uint8_t rxbuf[sizeof(tagged) + 4]; + struct wolfIP_sockaddr_ll from; + socklen_t from_len; + int ret; + + setup_vlan_stack(&s); + ret = wolfIP_vlan_create(&s, TEST_PRIMARY_IF, 100, 0, 0, &sub_idx); + ck_assert_int_eq(ret, 0); + phys = wolfIP_getdev_ex(&s, TEST_PRIMARY_IF); + ck_assert_ptr_nonnull(phys); + + /* protocol 0 == ETH_P_ALL so both the 0x8100 (tagged) and 0x0800 + * (stripped) views match. */ + sd_parent = wolfIP_sock_socket(&s, AF_PACKET, IPSTACK_SOCK_RAW, 0); + ck_assert_int_ge(sd_parent, 0); + s.packetsockets[SOCKET_UNMARK(sd_parent)].bind_addr.sll_ifindex = + (int)TEST_PRIMARY_IF; + sd_sub = wolfIP_sock_socket(&s, AF_PACKET, IPSTACK_SOCK_RAW, 0); + ck_assert_int_ge(sd_sub, 0); + s.packetsockets[SOCKET_UNMARK(sd_sub)].bind_addr.sll_ifindex = (int)sub_idx; + + plain_len = build_icmp_echo_request(plain, sizeof(plain), phys->mac, + VLAN_REMOTE_IP, VLAN_SUB100_IP); + ck_assert_uint_gt(plain_len, 0u); + tagged_len = insert_vlan_tag(tagged, sizeof(tagged), plain, plain_len, + 100, 0, 0); + ck_assert_uint_gt(tagged_len, 0u); + + wolfIP_recv_on(&s, TEST_PRIMARY_IF, tagged, tagged_len); + + /* Parent-bound socket: original tagged frame, stamped with the parent. */ + memset(&from, 0, sizeof(from)); + from_len = sizeof(from); + ret = wolfIP_sock_recvfrom(&s, sd_parent, rxbuf, sizeof(rxbuf), 0, + (struct wolfIP_sockaddr *)&from, &from_len); + ck_assert_int_eq(ret, (int)tagged_len); + ck_assert_uint_eq(rxbuf[12], 0x81u); /* VLAN TPID still present */ + ck_assert_uint_eq(rxbuf[13], 0x00u); + ck_assert_int_eq(from.sll_ifindex, (int)TEST_PRIMARY_IF); + + /* Sub-interface-bound socket: tag-stripped frame, stamped with the sub. */ + memset(&from, 0, sizeof(from)); + from_len = sizeof(from); + ret = wolfIP_sock_recvfrom(&s, sd_sub, rxbuf, sizeof(rxbuf), 0, + (struct wolfIP_sockaddr *)&from, &from_len); + ck_assert_int_eq(ret, (int)(tagged_len - 4)); + ck_assert_uint_eq(rxbuf[12], 0x08u); /* inner IPv4 ethertype, tag gone */ + ck_assert_uint_eq(rxbuf[13], 0x00u); + ck_assert_int_eq(from.sll_ifindex, (int)sub_idx); +} +END_TEST + +/* F-5011: a wildcard (sll_ifindex == 0) sniffer must receive the original + * tagged frame exactly once - stamped with the parent - not the tag-stripped + * sub-interface copy, and not both. */ +START_TEST(test_vlan_packet_socket_wildcard_gets_tagged_once) +{ + struct wolfIP s; + struct wolfIP_ll_dev *phys; + unsigned int sub_idx = 0xFFFFFFFFu; + int sd; + uint8_t plain[ETH_HEADER_LEN + IP_HEADER_LEN + ICMP_HEADER_LEN]; + uint8_t tagged[ETH_HEADER_LEN + IP_HEADER_LEN + ICMP_HEADER_LEN + 4]; + uint32_t plain_len, tagged_len; + uint8_t rxbuf[sizeof(tagged) + 4]; + struct wolfIP_sockaddr_ll from; + socklen_t from_len; + int ret; + + setup_vlan_stack(&s); + ret = wolfIP_vlan_create(&s, TEST_PRIMARY_IF, 100, 0, 0, &sub_idx); + ck_assert_int_eq(ret, 0); + phys = wolfIP_getdev_ex(&s, TEST_PRIMARY_IF); + ck_assert_ptr_nonnull(phys); + + sd = wolfIP_sock_socket(&s, AF_PACKET, IPSTACK_SOCK_RAW, 0); + ck_assert_int_ge(sd, 0); + s.packetsockets[SOCKET_UNMARK(sd)].bind_addr.sll_ifindex = 0; /* wildcard */ + + plain_len = build_icmp_echo_request(plain, sizeof(plain), phys->mac, + VLAN_REMOTE_IP, VLAN_SUB100_IP); + ck_assert_uint_gt(plain_len, 0u); + tagged_len = insert_vlan_tag(tagged, sizeof(tagged), plain, plain_len, + 100, 0, 0); + ck_assert_uint_gt(tagged_len, 0u); + + wolfIP_recv_on(&s, TEST_PRIMARY_IF, tagged, tagged_len); + + /* First (and only) delivery: the tagged wire frame on the parent. */ + memset(&from, 0, sizeof(from)); + from_len = sizeof(from); + ret = wolfIP_sock_recvfrom(&s, sd, rxbuf, sizeof(rxbuf), 0, + (struct wolfIP_sockaddr *)&from, &from_len); + ck_assert_int_eq(ret, (int)tagged_len); + ck_assert_uint_eq(rxbuf[12], 0x81u); + ck_assert_int_eq(from.sll_ifindex, (int)TEST_PRIMARY_IF); + + /* No duplicate tag-stripped copy. */ + ret = wolfIP_sock_recvfrom(&s, sd, rxbuf, sizeof(rxbuf), 0, NULL, 0); + ck_assert_int_eq(ret, -WOLFIP_EAGAIN); +} +END_TEST +#endif /* WOLFIP_PACKET_SOCKETS */ + START_TEST(test_vlan_rx_runt_tagged_dropped) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 06c59df5..86ba848b 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -2627,7 +2627,7 @@ static void raw_try_recv(struct wolfIP *s, unsigned int if_idx, struct wolfIP_ip #endif #if WOLFIP_PACKET_SOCKETS -static void packet_try_recv(struct wolfIP *s, unsigned int if_idx, struct wolfIP_eth_frame *eth, uint32_t frame_len) +static void packet_try_recv(struct wolfIP *s, unsigned int if_idx, struct wolfIP_eth_frame *eth, uint32_t frame_len, int match_wildcard) { uint16_t proto = eth->type; uint32_t record_len; @@ -2646,10 +2646,22 @@ static void packet_try_recv(struct wolfIP *s, unsigned int if_idx, struct wolfIP continue; if (p->protocol && p->protocol != proto) continue; - if ((p->bind_addr.sll_ifindex >= 0) && - (unsigned int)p->bind_addr.sll_ifindex != if_idx && - p->bind_addr.sll_ifindex != 0) - continue; + if (match_wildcard) { + /* Deliver to sockets bound to this interface, plus wildcard + * (sll_ifindex == 0) and unbound (sll_ifindex < 0) listeners. */ + if ((p->bind_addr.sll_ifindex >= 0) && + (unsigned int)p->bind_addr.sll_ifindex != if_idx && + p->bind_addr.sll_ifindex != 0) + continue; + } else { + /* Deliver only to sockets bound explicitly to this interface. + * Used for the post-VLAN-demux pass so wildcard sockets, which + * already saw the original tagged frame on the parent, are not + * delivered the tag-stripped copy a second time. */ + if (p->bind_addr.sll_ifindex < 0 || + (unsigned int)p->bind_addr.sll_ifindex != if_idx) + continue; + } if (fifo_space(&p->rxbuf) < record_len + sizeof(struct pkt_desc)) continue; if (fifo_push(&p->rxbuf, record, record_len) == 0) @@ -8855,6 +8867,9 @@ static void wolfIP_recv_on(struct wolfIP *s, unsigned int if_idx, void *buf, uin #ifdef ETHERNET struct wolfIP_ll_dev *ll; struct wolfIP_eth_frame *eth; +#if WOLFIP_PACKET_SOCKETS + int pkt_match_wildcard = 1; +#endif #else struct wolfIP_ip_packet *ip = (struct wolfIP_ip_packet *)buf; #endif @@ -8900,6 +8915,15 @@ static void wolfIP_recv_on(struct wolfIP *s, unsigned int if_idx, void *buf, uin } if (!found) return; /* No matching VLAN sub-interface; drop. */ +#if WOLFIP_PACKET_SOCKETS + /* Tap the physical (parent) interface with the original, still-tagged + * frame so AF_PACKET listeners bound to the parent - and wildcard + * sniffers/IDS - observe VLAN traffic in wire form. The post-demux + * delivery below then targets only sockets bound explicitly to the + * sub-interface, so wildcard sockets are not given the frame twice. */ + packet_try_recv(s, if_idx, eth, len, 1); + pkt_match_wildcard = 0; +#endif /* Strip the 4-byte tag in place: slide MAC headers forward. */ memmove((uint8_t *)buf + WOLFIP_VLAN_TAG_LEN, buf, 12); buf = (uint8_t *)buf + WOLFIP_VLAN_TAG_LEN; @@ -8911,7 +8935,7 @@ static void wolfIP_recv_on(struct wolfIP *s, unsigned int if_idx, void *buf, uin } #endif /* WOLFIP_VLAN */ #if WOLFIP_PACKET_SOCKETS - packet_try_recv(s, if_idx, eth, len); + packet_try_recv(s, if_idx, eth, len, pkt_match_wildcard); #endif if (eth->type == ee16(ETH_TYPE_IP)) { struct wolfIP_ip_packet *ip = (struct wolfIP_ip_packet *)eth; From 16e20dfa5e34074997b93611bd597d6561ecd4ca Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 12:07:06 +0200 Subject: [PATCH 08/24] F-5697: never forward a packet whose source is one of our own IPs The strict-RPF loop in ip_recv's forwarding block skips the ingress interface (i == if_idx), so a source address equal to the router's own IP on that interface was never checked. An L2-adjacent attacker could send a packet with src set to the router's own LAN address and have it forwarded out another interface with the source unchanged, impersonating the router to downstream hosts. (Other interfaces' own IPs were already caught by the RPF subnet match, but the ingress interface's own /32 slipped through.) Add an explicit anti-spoof check before the strict-RPF loop: drop the packet if its source equals any configured interface address. The router originates such packets locally and never legitimately receives them from the wire. The check matches only the exact own /32, so legitimate hosts in the ingress subnet are still forwarded. Two existing forwarding tests used the router's own IP as a convenience source; they are updated to a real in-subnet host so they keep exercising the TTL- exceeded and ARP-queue forward paths. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_ip_arp_recv.c | 60 ++++++++++++++++++++++++++ src/test/unit/unit_tests_tcp_ack.c | 8 +++- src/wolfip.c | 17 ++++++++ 4 files changed, 84 insertions(+), 2 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index a858f50a..74580574 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -1349,6 +1349,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_ip_recv_forward_arp_hit_sends_immediately); tcase_add_test(tc_core, test_ip_recv_forward_unconfigured_iface_skipped); tcase_add_test(tc_core, test_ip_recv_forward_link_local_src_rpf_drop); + tcase_add_test(tc_core, test_ip_recv_forward_self_ip_src_dropped); tcase_add_test(tc_core, test_ip_recv_options_nop_delivered); tcase_add_test(tc_core, test_ip_recv_options_rr_stripped_and_delivered); tcase_add_test(tc_core, test_ip_recv_options_bad_length_aborts_parse); diff --git a/src/test/unit/unit_tests_ip_arp_recv.c b/src/test/unit/unit_tests_ip_arp_recv.c index f0407a77..18a2771b 100644 --- a/src/test/unit/unit_tests_ip_arp_recv.c +++ b/src/test/unit/unit_tests_ip_arp_recv.c @@ -321,6 +321,66 @@ START_TEST(test_ip_recv_forward_link_local_src_rpf_drop) } END_TEST +/* ========================================================================= + * F-5697: a packet whose source is the router's OWN IP on the ingress + * interface must never be forwarded. + * ========================================================================= + * The strict-RPF loop skips i == if_idx, so the ingress interface's own + * address was never compared against the source. An L2-adjacent attacker + * could spoof the router's LAN IP and have it forwarded out another + * interface. A legitimate host in the same ingress subnet must still be + * forwarded (the fix only matches the exact own /32, not the subnet). + */ +START_TEST(test_ip_recv_forward_self_ip_src_dropped) +{ + struct wolfIP s; + uint8_t frame[ETH_HEADER_LEN + IP_HEADER_LEN + UDP_HEADER_LEN]; + struct wolfIP_ip_packet *ip = (struct wolfIP_ip_packet *)frame; + ip4 primary_ip = 0x0A000001U; /* 10.0.0.1 — router LAN IP (if1) */ + ip4 secondary_ip = 0xC0A80101U; /* 192.168.1.1 — router WAN IP (if2) */ + ip4 dest_ip = 0xC0A80155U; /* 192.168.1.85 — local to if2 */ + static const uint8_t dest_mac[6] = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15}; + + setup_stack_with_two_ifaces(&s, primary_ip, secondary_ip); + wolfIP_filter_set_callback(NULL, NULL); + /* ARP hit so that, absent the drop, the packet would be forwarded now. */ + arp_store_neighbor(&s, TEST_SECOND_IF, dest_ip, dest_mac); + + memset(frame, 0, sizeof(frame)); + memcpy(ip->eth.dst, s.ll_dev[TEST_PRIMARY_IF].mac, 6); + memcpy(ip->eth.src, "\x01\x02\x03\x04\x05\x06", 6); + ip->eth.type = ee16(ETH_TYPE_IP); + ip->ver_ihl = 0x45; + ip->ttl = 64; + ip->proto = WI_IPPROTO_UDP; + ip->len = ee16(IP_HEADER_LEN + UDP_HEADER_LEN); + ip->src = ee32(primary_ip); /* spoof: the router's own LAN IP */ + ip->dst = ee32(dest_ip); + fix_ip_checksum(ip); + { + uint16_t *udp = (uint16_t *)(frame + ETH_HEADER_LEN + IP_HEADER_LEN); + udp[0] = ee16(9999); udp[1] = ee16(53); + udp[2] = ee16(UDP_HEADER_LEN); udp[3] = 0; + } + + last_frame_sent_size = 0; + ip_recv(&s, TEST_PRIMARY_IF, ip, (uint32_t)sizeof(frame)); + + /* Self-IP source must be dropped: nothing forwarded, nothing queued. */ + ck_assert_uint_eq(last_frame_sent_size, 0); + ck_assert_uint_eq(s.arp_pending[0].dest, IPADDR_ANY); + + /* Sanity: a real host in the same ingress subnet is still forwarded + * (the drop targets the exact own address, not the whole subnet). */ + ip->src = ee32(0x0A000002U); /* 10.0.0.2 — legitimate LAN host */ + fix_ip_checksum(ip); + last_frame_sent_size = 0; + ip_recv(&s, TEST_PRIMARY_IF, ip, (uint32_t)sizeof(frame)); + ck_assert_uint_gt(last_frame_sent_size, 0); + ck_assert_mem_eq(last_frame_sent + 0, dest_mac, 6); +} +END_TEST + /* ========================================================================= * ip_recv: IP with NOP options — options parsed, payload delivered * ========================================================================= diff --git a/src/test/unit/unit_tests_tcp_ack.c b/src/test/unit/unit_tests_tcp_ack.c index 5b674a6d..eb07c311 100644 --- a/src/test/unit/unit_tests_tcp_ack.c +++ b/src/test/unit/unit_tests_tcp_ack.c @@ -1410,7 +1410,9 @@ START_TEST(test_ip_recv_forward_ttl_exceeded) ip->ttl = 1; ip->proto = WI_IPPROTO_UDP; ip->len = ee16(IP_HEADER_LEN + 8); - ip->src = ee32(primary_ip); + /* Legitimate in-subnet host source (not the router's own IP, which is + * now dropped as a spoof on the forward path - see F-5697). */ + ip->src = ee32(0x0A000002U); ip->dst = ee32(0xC0A80155U); fix_ip_checksum(ip); @@ -1586,7 +1588,9 @@ START_TEST(test_ip_recv_forward_arp_queue_and_flush) ip.ttl = 2; ip.proto = WI_IPPROTO_TCP; ip.len = ee16(IP_HEADER_LEN); - ip.src = ee32(primary_ip); + /* Legitimate in-subnet host source (not the router's own IP, which is + * now dropped as a spoof on the forward path - see F-5697). */ + ip.src = ee32(0x0A000002U); ip.dst = ee32(dest_ip); fix_ip_checksum(&ip); diff --git a/src/wolfip.c b/src/wolfip.c index 86ba848b..8208d726 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -8736,6 +8736,23 @@ static inline void ip_recv(struct wolfIP *s, unsigned int if_idx, if (!rpf_drop && (src & 0xFFFF0000U) == 0xA9FE0000U) { rpf_drop = 1; } + /* Spoofed self: a source equal to one of our own configured + * interface addresses can only be forged - the router originates + * such packets locally, it never receives them from the wire. The + * strict-RPF loop below skips the ingress interface (i == if_idx), + * so its own address would otherwise pass; check every interface's + * own /32 here explicitly. */ + if (!rpf_drop) { + for (i = 0; i < s->if_count; i++) { + struct ipconf *conf = &s->ipconf[i]; + if (!conf || conf->ip == IPADDR_ANY) + continue; + if (conf->ip == src) { + rpf_drop = 1; + break; + } + } + } /* Strict RPF: a source that belongs to some other configured * interface's local subnet must not arrive on this one. */ if (!rpf_drop) { From b9e84e39eb396c9d1b62d7fd95a9a7c16eddc74a Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 12:17:38 +0200 Subject: [PATCH 09/24] F-5784: strip IP options before ESP unwrap so IHL>5 ESP packets are handled ip_recv dispatched proto=50 (ESP) to esp_transport_unwrap before the IP option-strip block. esp_transport_unwrap reads the ESP header (SPI, sequence, ICV, payload) at a fixed 20-byte-IP-header offset (ip->data) and derives esp_len from a fixed IP_HEADER_LEN. When the outer IPv4 packet carried options (IHL>5, e.g. Router-Alert), ip->data pointed into the options region: the SPI was read from option bytes, the SA lookup failed, and every such ESP packet was silently dropped regardless of SA registration. Move the ESP unwrap into the dispatch block, after the existing option-strip that normalizes the header to IHL=5. esp_transport_unwrap now always receives a 20-byte IP header, so the ESP header is at the offset it expects. Non-ESP and non-option paths are unchanged (the strip and protocol dispatch are identical; only the ESP step moved down). --- src/test/unit/unit_esp.c | 62 ++++++++++++++++++++++++++++++++++++++++ src/wolfip.c | 39 ++++++++++++++----------- 2 files changed, 84 insertions(+), 17 deletions(-) diff --git a/src/test/unit/unit_esp.c b/src/test/unit/unit_esp.c index 1dbe906d..80a6fe86 100644 --- a/src/test/unit/unit_esp.c +++ b/src/test/unit/unit_esp.c @@ -1388,6 +1388,67 @@ START_TEST(test_ip_recv_esp_transport_unwrap_failure_drops_packet) } END_TEST +/* F-5784: an ESP packet whose outer IPv4 header carries options (IHL>5) must + * still be unwrapped and delivered. ip_recv used to dispatch ESP before the + * IP option-strip, so esp_transport_unwrap read the SPI from the option bytes, + * the SA lookup failed, and every IHL>5 ESP packet was silently dropped. */ +START_TEST(test_ip_recv_esp_transport_with_ip_options_delivers_payload) +{ + static uint8_t buf[LINK_MTU + 256]; + struct wolfIP s; + struct wolfIP_ip_packet *ip = (struct wolfIP_ip_packet *)buf; + struct wolfIP_sockaddr_in sin; + uint8_t payload[] = { 'o', 'p', 't', '!' }; + uint8_t rxbuf[sizeof(payload)] = {0}; + uint32_t frame_len; + uint16_t ip_len; + uint32_t esp_payload_len; + uint8_t *opt; + int udp_sd; + int ret; + + wolfIP_init(&s); + esp_setup(); + esp_add_cbc_test_sas(); + wolfIP_ipconfig_set(&s, atoip4(T_DST), 0xFFFFFF00U, 0); + + udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); + ck_assert_int_gt(udp_sd, 0); + + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_port = ee16(1234); + sin.sin_addr.s_addr = ee32(atoip4(T_DST)); + ck_assert_int_eq(wolfIP_sock_bind(&s, udp_sd, (struct wolfIP_sockaddr *)&sin, sizeof(sin)), 0); + + frame_len = build_udp_ip_packet(buf, sizeof(buf), atoip4(T_SRC), atoip4(T_DST), + 4321, 1234, payload, sizeof(payload)); + ip_len = (uint16_t)(frame_len - ETH_HEADER_LEN); + + ret = esp_transport_wrap(ip, &ip_len); + ck_assert_int_eq(ret, 0); + frame_len = (uint32_t)ip_len + ETH_HEADER_LEN; + + /* Splice a 4-byte Router-Alert option into the outer IP header so it + * becomes IHL=6, shifting the ESP header off its default offset. */ + esp_payload_len = frame_len - (ETH_HEADER_LEN + IP_HEADER_LEN); + opt = buf + ETH_HEADER_LEN + IP_HEADER_LEN; + memmove(opt + 4U, opt, esp_payload_len); + opt[0] = 0x94U; opt[1] = 0x04U; opt[2] = 0x00U; opt[3] = 0x00U; + ip->ver_ihl = 0x46U; + ip->len = ee16((uint16_t)(ee16(ip->len) + 4U)); + ip->csum = 0; + iphdr_set_checksum(ip); + frame_len += 4U; + + ip_recv(&s, 0, ip, frame_len); + + ret = wolfIP_sock_recvfrom(&s, udp_sd, rxbuf, sizeof(rxbuf), 0, NULL, NULL); + ck_assert_int_eq(ret, (int)sizeof(payload)); + ck_assert_mem_eq(rxbuf, payload, sizeof(payload)); +} +END_TEST + /* Mock send that captures the last frame sent. * Used by tests that exercise the full TX path (tcp_send_empty_immediate). */ static uint8_t esp_test_last_frame[LINK_MTU]; @@ -1756,6 +1817,7 @@ static Suite *esp_suite(void) tc = tcase_create("ip_recv"); tcase_add_test(tc, test_ip_recv_esp_transport_delivers_udp_payload); tcase_add_test(tc, test_ip_recv_esp_transport_unwrap_failure_drops_packet); + tcase_add_test(tc, test_ip_recv_esp_transport_with_ip_options_delivers_payload); suite_add_tcase(s, tc); /* No-SA outbound path */ diff --git a/src/wolfip.c b/src/wolfip.c index 8208d726..02717a78 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -8806,23 +8806,6 @@ static inline void ip_recv(struct wolfIP *s, unsigned int if_idx, wolfIP_print_ip(ip); #endif /* DEBUG_IP*/ - #ifdef WOLFIP_ESP - /* note: esp transport mode only handled here. - * ip forwarding would require esp tunnel mode. */ - if (ip->proto == 0x32) { - int err; - if (wolfIP_ll_is_non_ethernet(s, if_idx)) { - return; - } - /* proto is ESP 0x32 (50), try to unwrap. */ - err = esp_transport_unwrap(ip, &len); - if (err) { - LOG("info: failed to unwrap esp packet, dropping.\n"); - return; - } - } - #endif /* WOLFIP_ESP */ - { struct wolfIP_ip_packet *dispatch_ip = ip; uint32_t dispatch_len = len; @@ -8846,6 +8829,28 @@ static inline void ip_recv(struct wolfIP *s, unsigned int if_idx, iphdr_set_checksum(dispatch_ip); } + #ifdef WOLFIP_ESP + /* note: esp transport mode only handled here. + * ip forwarding would require esp tunnel mode. + * Run after the option strip above: esp_transport_unwrap reads the + * ESP header at a fixed 20-byte-IP-header offset, so it must be given + * a packet whose options have already been removed (IHL == 5). + * Otherwise an IHL>5 ESP packet has its SPI read from the option + * bytes, the SA lookup fails, and it is silently dropped. */ + if (dispatch_ip->proto == 0x32) { + int err; + if (wolfIP_ll_is_non_ethernet(s, if_idx)) { + return; + } + /* proto is ESP 0x32 (50), try to unwrap. */ + err = esp_transport_unwrap(dispatch_ip, &dispatch_len); + if (err) { + LOG("info: failed to unwrap esp packet, dropping.\n"); + return; + } + } + #endif /* WOLFIP_ESP */ + if (dispatch_ip->proto == 0x06) { struct wolfIP_tcp_seg *tcp = (struct wolfIP_tcp_seg *)dispatch_ip; tcp_input(s, if_idx, tcp, dispatch_len); From 435a976d77d20a789d708b855909387b8429c135 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 12:35:05 +0200 Subject: [PATCH 10/24] F-5484: document that raw sockets intentionally tap before source-route policy ip_recv delivers to raw_try_recv before the RFC 7126 LSRR/SSRR source-route drop, so raw sockets observe source-routed packets the stack later refuses to forward or deliver. This was flagged as a possible policy-ordering issue, but it is the intended and correct behaviour: a raw socket is a passive ingress tap whose purpose is to see wire traffic - including hostile packets the stack itself rejects - which is exactly what monitoring/IDS use cases need. Moving the scan ahead of the tap would blind those listeners to source-routed attacks. raw_try_recv only copies the frame to the socket queue; it never parses options or honours a source route, so tapping early cannot cause the stack to act on one. Add a comment recording this intent (the alternative the report offered) rather than changing behaviour. No functional change. --- src/wolfip.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/wolfip.c b/src/wolfip.c index 02717a78..5247e9e8 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -8679,6 +8679,12 @@ static inline void ip_recv(struct wolfIP *s, unsigned int if_idx, if (wolfIP_filter_notify_ip(WOLFIP_FILT_RECEIVING, s, if_idx, ip, len) != 0) return; #if WOLFIP_RAWSOCKETS + /* Raw sockets are a passive ingress tap and intentionally observe traffic + * before the option/forwarding policy below: this is what makes them + * useful for monitoring/IDS (e.g. seeing source-routed attack packets the + * stack itself refuses to act on). raw_try_recv only copies the frame to + * the socket queue; it never parses IP options or honours a source route, + * so tapping ahead of the LSRR/SSRR drop cannot make the stack act on one. */ raw_try_recv(s, if_idx, ip, len); #endif /* RFC 7126 section 3.8: drop source-routed (LSRR/SSRR) packets before either From 1aa3587f0a77b887701d01643cec10822610d255 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 12:53:12 +0200 Subject: [PATCH 11/24] F-4948: purge ARP state for a VLAN slot on delete to stop cross-VLAN L2 trust wolfIP_vlan_delete wiped ll_dev[if_idx] and ipconf[if_idx] but left the ARP neighbor cache, the in-flight ARP request tracker, the queued-packet array and last_arp[if_idx] untouched. ARP neighbor entries are keyed only by (ip, if_idx) - there is no VID, generation counter, or liveness tie to the interface - and wolfIP_vlan_create immediately reuses the freed slot. So a new VLAN on the same if_idx silently inherited the deleted VLAN's IP->MAC mappings for up to ARP_AGING_TIMEOUT_MS, breaking per-VLAN L2 isolation without any ARP exchange on the new VID. After wiping the slot, evict every arp.neighbors[] and arp.pending[] / arp_pending[] entry whose if_idx matches the deleted slot and clear last_arp[if_idx], so the reused slot starts from a clean ARP state. Note: the report's dedupe group mentions a sibling multicast-membership variant (f_vlan_stale_mcast_1); that is a separate finding and is not touched here. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_vlan.c | 35 +++++++++++++++++++++++++++++++++ src/wolfip.c | 28 ++++++++++++++++++++++++++ 3 files changed, 64 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 74580574..f37cd707 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -1530,6 +1530,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_vlan_rx_dei_bit_accepted); tcase_add_test(tc_proto, test_vlan_rx_tagged_arp_processed); tcase_add_test(tc_proto, test_vlan_mtu_inherited); + tcase_add_test(tc_proto, test_vlan_delete_purges_arp_neighbor_cache); #if WOLFIP_PACKET_SOCKETS tcase_add_test(tc_proto, test_vlan_packet_socket_parent_and_sub_delivery); tcase_add_test(tc_proto, test_vlan_packet_socket_wildcard_gets_tagged_once); diff --git a/src/test/unit/unit_tests_vlan.c b/src/test/unit/unit_tests_vlan.c index f9bf6cf4..382807ed 100644 --- a/src/test/unit/unit_tests_vlan.c +++ b/src/test/unit/unit_tests_vlan.c @@ -1306,4 +1306,39 @@ START_TEST(test_vlan_mtu_inherited) } END_TEST +/* F-4948: deleting a VLAN sub-interface must purge the ARP neighbor cache for + * that slot. ARP entries are keyed only by (ip, if_idx) with no VID, and + * wolfIP_vlan_create reuses the freed slot, so without a purge a new VLAN on + * the same if_idx would inherit the deleted VLAN's L2 mappings. */ +START_TEST(test_vlan_delete_purges_arp_neighbor_cache) +{ + struct wolfIP s; + unsigned int sub100 = 0xFFFFFFFFu, sub200 = 0xFFFFFFFFu; + static const uint8_t peer_mac[6] = {0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF}; + const ip4 peer_ip = VLAN_REMOTE_IP; + uint8_t mac[6]; + int ret; + + setup_vlan_stack(&s); + + /* Learn a neighbor on a VID=100 sub-interface. */ + ret = wolfIP_vlan_create(&s, TEST_PRIMARY_IF, 100, 0, 0, &sub100); + ck_assert_int_eq(ret, 0); + arp_store_neighbor(&s, sub100, peer_ip, peer_mac); + ck_assert_int_eq(wolfIP_arp_lookup_ex(&s, sub100, peer_ip, mac), 0); + ck_assert_mem_eq(mac, peer_mac, 6); + + /* Delete VID=100 and recreate as VID=200 on the same slot. */ + ck_assert_int_eq(wolfIP_vlan_delete(&s, sub100), 0); + ret = wolfIP_vlan_create(&s, TEST_PRIMARY_IF, 200, 0, 0, &sub200); + ck_assert_int_eq(ret, 0); + ck_assert_uint_eq(sub200, sub100); /* slot reused */ + + /* The new VLAN must not inherit the deleted VLAN's L2 mapping. */ + memset(mac, 0, sizeof(mac)); + ret = wolfIP_arp_lookup_ex(&s, sub200, peer_ip, mac); + ck_assert_int_eq(ret, -1); +} +END_TEST + #endif /* WOLFIP_VLAN */ diff --git a/src/wolfip.c b/src/wolfip.c index 5247e9e8..14fbcd06 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -8538,6 +8538,34 @@ int wolfIP_vlan_delete(struct wolfIP *s, unsigned int if_idx) * renumbering active sub-ifaces. */ memset(slot, 0, sizeof(*slot)); memset(&s->ipconf[if_idx], 0, sizeof(s->ipconf[if_idx])); + /* Purge ARP state tied to this slot. Neighbor entries are keyed only by + * (ip, if_idx) with no VID, and wolfIP_vlan_create reuses the freed slot, + * so without this a new VLAN on the same if_idx would silently inherit the + * deleted VLAN's L2 mappings (and its queued/in-flight ARP state). */ + { + int i; + for (i = 0; i < MAX_NEIGHBORS; i++) { + if (s->arp.neighbors[i].if_idx == (uint8_t)if_idx) { + s->arp.neighbors[i].ip = IPADDR_ANY; + s->arp.neighbors[i].if_idx = 0; + s->arp.neighbors[i].ts = 0; + memset(s->arp.neighbors[i].mac, 0, 6); + } + } + for (i = 0; i < WOLFIP_ARP_PENDING_MAX; i++) { + if (s->arp.pending[i].if_idx == (uint8_t)if_idx) { + s->arp.pending[i].ip = IPADDR_ANY; + s->arp.pending[i].if_idx = 0; + s->arp.pending[i].ts = 0; + } + if (s->arp_pending[i].if_idx == (uint8_t)if_idx) { + s->arp_pending[i].dest = IPADDR_ANY; + s->arp_pending[i].len = 0; + s->arp_pending[i].if_idx = 0; + } + } + s->arp.last_arp[if_idx] = 0; + } return 0; } From 129d267ce8d854b6a6c7ab6b00dbbf6a025b3d76 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 14:25:06 +0200 Subject: [PATCH 12/24] F-5698: re-randomize the DHCP xid for each renewal cycle dhcp_xid was assigned once in dhcp_client_init and reused verbatim for every T1/T2 renewal DHCPREQUEST. Both the xid and the server-id are observable from the initial broadcast DORA exchange, and dhcp_parse_ack accepts a DHCPACK on matching xid + server-id while overwriting the gateway unconditionally. Since the RENEWING DHCPREQUEST is unicast to the server, a LAN-adjacent attacker who captured the initial DORA could blindly forge a renewal DHCPACK (same xid, real server-id, attacker-controlled router option) at every renewal without ever seeing the renewal request, redirecting routed traffic through their host. Generate a fresh wolfIP_getrandom() xid at the BOUND->RENEWING transition, so each renewal is a new transaction with an unpredictable id (RFC 2131). Retransmissions within the cycle and the RENEWING->REBINDING continuation keep the xid, as required. An attacker who only saw the initial DORA can no longer predict the renewal xid, so a replayed-xid ACK is rejected. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_dhcp_edges.c | 60 +++++++++++++++++++++++++++ src/wolfip.c | 6 +++ 3 files changed, 67 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index f37cd707..bca1344b 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -1342,6 +1342,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_dhcp_timer_cb_bound_lease_not_expired_starts_renew); tcase_add_test(tc_core, test_dhcp_timer_cb_default_state_noop); tcase_add_test(tc_core, test_dhcp_timer_cb_null_arg_noop); + tcase_add_test(tc_core, test_dhcp_renew_rerandomizes_xid_rejecting_stale_ack); /* --- unit_tests_ip_arp_recv.c (34 tests) --- */ tcase_add_test(tc_core, test_ip_recv_limited_broadcast_dst_is_local); tcase_add_test(tc_core, test_ip_recv_directed_broadcast_dst_is_local); diff --git a/src/test/unit/unit_tests_dhcp_edges.c b/src/test/unit/unit_tests_dhcp_edges.c index 3c3cd68a..bdffcf67 100644 --- a/src/test/unit/unit_tests_dhcp_edges.c +++ b/src/test/unit/unit_tests_dhcp_edges.c @@ -1127,3 +1127,63 @@ START_TEST(test_dhcp_timer_cb_null_arg_noop) dhcp_timer_cb(NULL); } END_TEST + +/* F-5698: each DHCP renewal cycle must use a fresh transaction ID. The xid and + * server-id are observable from the initial (broadcast) DORA exchange; if the + * renewal DHCPREQUEST reused that xid, a LAN-adjacent attacker could blindly + * forge a renewal DHCPACK with the captured xid and hijack the gateway without + * ever seeing the (unicast) renewal request. */ +START_TEST(test_dhcp_renew_rerandomizes_xid_rejecting_stale_ack) +{ + struct wolfIP s; + struct dhcp_msg msg; + struct ipconf *primary; + const uint32_t old_xid = 0xDEADBEEFU; /* captured from initial DORA */ + const uint32_t new_xid = 0xC0FFEE11U; /* fresh xid for the renewal */ + const uint32_t server_ip = 0x0A000001U; + const uint32_t good_gw = 0x0A000001U; + const uint32_t attacker_gw = 0x0A0000FEU; + + wolfIP_init(&s); + mock_link_init(&s); + primary = wolfIP_primary_ipconf(&s); + ck_assert_ptr_nonnull(primary); + primary->ip = 0x0A000064U; + primary->mask = 0xFFFFFF00U; + primary->gw = good_gw; + s.dhcp_server_ip = server_ip; + s.dhcp_ip = primary->ip; + s.dhcp_xid = old_xid; + s.dhcp_state = DHCP_BOUND; + s.dhcp_lease_expires = 0; /* not expired -> renew, do not deconfigure */ + s.dhcp_rebind_at = 2000U; + s.last_tick = 1000U; + s.dhcp_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, + WI_IPPROTO_UDP); + ck_assert_int_gt(s.dhcp_udp_sd, 0); + + /* T1 fires: the renewal cycle must adopt a fresh, unpredictable xid. */ + test_rand_override_enabled = 1; + test_rand_override_value = new_xid; + dhcp_timer_cb(&s); + test_rand_override_enabled = 0; + + ck_assert_int_eq(s.dhcp_state, DHCP_RENEWING); + ck_assert_uint_eq(s.dhcp_xid, new_xid); + ck_assert_uint_ne(s.dhcp_xid, old_xid); + + /* A forged ACK replaying the captured DORA xid must be rejected outright, + * leaving the gateway untouched. */ + build_full_ack(&s, &msg, server_ip, primary->ip, primary->mask, + attacker_gw, 0x08080808U, 120U); + msg.xid = ee32(old_xid); + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, sizeof(msg)), -1); + ck_assert_uint_eq(primary->gw, good_gw); + + /* The legitimate renewal ACK carrying the new xid is still accepted. */ + build_full_ack(&s, &msg, server_ip, primary->ip, primary->mask, + good_gw, 0x08080808U, 120U); + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, sizeof(msg)), 0); + ck_assert_uint_eq(primary->gw, good_gw); +} +END_TEST diff --git a/src/wolfip.c b/src/wolfip.c index 14fbcd06..0b2734e1 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -7457,6 +7457,12 @@ static void dhcp_timer_cb(void *arg) s->dhcp_state = DHCP_RENEWING; s->dhcp_start_tick = s->last_tick; s->dhcp_timeout_count = 0; + /* RFC 2131: a renewal is a new transaction. Pick a fresh, random + * transaction ID so an attacker who observed the initial (broadcast) + * DORA xid cannot blindly forge a renewal DHCPACK for the lifetime + * of the lease. Retransmissions within this cycle (and the + * RENEWING->REBINDING continuation) keep this xid. */ + s->dhcp_xid = wolfIP_getrandom(); ret = dhcp_send_request(s); if (ret >= 0) s->dhcp_timeout_count++; From 71e81b8aadd5807e90f9d7003cf3bc1565ab5db0 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 16:35:46 +0200 Subject: [PATCH 13/24] F-5482: reject a DHCPACK that lacks the mandatory lease-time option dhcp_parse_ack initialized lease_s to 0 and transitioned to DHCP_BOUND without checking that option 51 (IP Address Lease Time) was present. RFC 2131 makes this option mandatory in a DHCPACK. With it absent, dhcp_schedule_lease_timer no-ops on lease_s == 0, so the client binds with no renewal/rebind/expiry timer and treats a dynamic lease as permanent - it never renews and never notices the server reclaiming the address. Gate the BOUND transition on lease_s != 0. lease_s is only ever assigned by the LEASE_TIME option (and a <4-byte option already returns -1 earlier), so the check accepts only an ACK carrying a present, nonzero lease time and rejects both the missing and zero-duration cases; a rejected ACK is ignored and the client keeps waiting/retransmitting. Several existing tests built DHCPACK fixtures without a lease-time option and relied on the prior (non-compliant) acceptance; they are updated to include the mandatory option so they keep exercising their actual intent (offer/ack flow, renewing, rebinding, unknown-option skipping). --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_dhcp_edges.c | 51 +++++++++++++++++++++++++++ src/test/unit/unit_tests_dns_dhcp.c | 15 ++++++++ src/test/unit/unit_tests_tcp_ack.c | 12 +++++-- src/wolfip.c | 7 +++- 5 files changed, 83 insertions(+), 3 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index bca1344b..fa891727 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -1343,6 +1343,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_dhcp_timer_cb_default_state_noop); tcase_add_test(tc_core, test_dhcp_timer_cb_null_arg_noop); tcase_add_test(tc_core, test_dhcp_renew_rerandomizes_xid_rejecting_stale_ack); + tcase_add_test(tc_core, test_dhcp_parse_ack_without_lease_time_rejected); /* --- unit_tests_ip_arp_recv.c (34 tests) --- */ tcase_add_test(tc_core, test_ip_recv_limited_broadcast_dst_is_local); tcase_add_test(tc_core, test_ip_recv_directed_broadcast_dst_is_local); diff --git a/src/test/unit/unit_tests_dhcp_edges.c b/src/test/unit/unit_tests_dhcp_edges.c index bdffcf67..087677c5 100644 --- a/src/test/unit/unit_tests_dhcp_edges.c +++ b/src/test/unit/unit_tests_dhcp_edges.c @@ -1187,3 +1187,54 @@ START_TEST(test_dhcp_renew_rerandomizes_xid_rejecting_stale_ack) ck_assert_uint_eq(primary->gw, good_gw); } END_TEST + +/* F-5482: a DHCPACK is only valid with the mandatory IP-address-lease-time + * option (51, RFC 2131). An ACK missing it - or carrying a zero duration - + * must be rejected, never bound, otherwise the lease has no expiry/renewal + * timer and is silently treated as permanent. */ +START_TEST(test_dhcp_parse_ack_without_lease_time_rejected) +{ + struct wolfIP s; + struct dhcp_msg msg; + struct ipconf *primary; + uint8_t *p; + + wolfIP_init(&s); + s.dhcp_xid = 0x5482U; + s.dhcp_server_ip = 0x0A000001U; + s.dhcp_state = DHCP_REQUEST_SENT; + primary = wolfIP_primary_ipconf(&s); + ck_assert_ptr_nonnull(primary); + + /* (1) ACK with server-id/offer-ip/mask/router but NO lease time. */ + build_dhcp_msg_base(&s, &msg, DHCP_ACK); + p = (uint8_t *)msg.options + 3; + append_opt4(&p, DHCP_OPTION_SERVER_ID, 0x0A000001U); + append_opt4(&p, DHCP_OPTION_OFFER_IP, 0x0A000064U); + append_opt4(&p, DHCP_OPTION_SUBNET_MASK, 0xFFFFFF00U); + append_opt4(&p, DHCP_OPTION_ROUTER, 0x0A000001U); + append_end(&p); + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, sizeof(msg)), -1); + ck_assert_int_ne(s.dhcp_state, DHCP_BOUND); + + /* (2) ACK that carries lease time == 0 is equally invalid. */ + build_dhcp_msg_base(&s, &msg, DHCP_ACK); + p = (uint8_t *)msg.options + 3; + append_opt4(&p, DHCP_OPTION_SERVER_ID, 0x0A000001U); + append_opt4(&p, DHCP_OPTION_OFFER_IP, 0x0A000064U); + append_opt4(&p, DHCP_OPTION_SUBNET_MASK, 0xFFFFFF00U); + append_opt4(&p, DHCP_OPTION_LEASE_TIME, 0U); + append_end(&p); + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, sizeof(msg)), -1); + ck_assert_int_ne(s.dhcp_state, DHCP_BOUND); + + /* (3) The same ACK with a valid nonzero lease time binds and schedules a + * lease timer. */ + build_full_ack(&s, &msg, 0x0A000001U, 0x0A000064U, 0xFFFFFF00U, + 0x0A000001U, 0x08080808U, 120U); + s.dhcp_timer = NO_TIMER; + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, sizeof(msg)), 0); + ck_assert_int_eq(s.dhcp_state, DHCP_BOUND); + ck_assert_int_ne(s.dhcp_timer, NO_TIMER); +} +END_TEST diff --git a/src/test/unit/unit_tests_dns_dhcp.c b/src/test/unit/unit_tests_dns_dhcp.c index cb9e79db..b2417569 100644 --- a/src/test/unit/unit_tests_dns_dhcp.c +++ b/src/test/unit/unit_tests_dns_dhcp.c @@ -71,6 +71,14 @@ static void build_dhcp_ack_msg(struct dhcp_msg *msg, uint32_t server_ip, uint32_ opt->data[2] = (dns_ip >> 8) & 0xFF; opt->data[3] = (dns_ip >> 0) & 0xFF; opt = (struct dhcp_option *)((uint8_t *)opt + 6); + /* The IP-address-lease-time option (51) is mandatory in a DHCPACK. */ + opt->code = DHCP_OPTION_LEASE_TIME; + opt->len = 4; + opt->data[0] = 0; + opt->data[1] = 0; + opt->data[2] = 0; + opt->data[3] = 120; + opt = (struct dhcp_option *)((uint8_t *)opt + 6); opt->code = DHCP_OPTION_END; opt->len = 0; } @@ -4753,6 +4761,13 @@ START_TEST(test_dhcp_poll_offer_and_ack) opt->data[2] = 0x08; opt->data[3] = 0x08; opt = (struct dhcp_option *)((uint8_t *)opt + 6); + opt->code = DHCP_OPTION_LEASE_TIME; /* mandatory in a DHCPACK */ + opt->len = 4; + opt->data[0] = 0x00; + opt->data[1] = 0x00; + opt->data[2] = 0x00; + opt->data[3] = 0x78; /* 120 s */ + opt = (struct dhcp_option *)((uint8_t *)opt + 6); opt->code = DHCP_OPTION_END; opt->len = 0; diff --git a/src/test/unit/unit_tests_tcp_ack.c b/src/test/unit/unit_tests_tcp_ack.c index eb07c311..7a545340 100644 --- a/src/test/unit/unit_tests_tcp_ack.c +++ b/src/test/unit/unit_tests_tcp_ack.c @@ -726,10 +726,14 @@ START_TEST(test_dhcp_parse_ack_ignores_short_unknown_option) opt->data[2] = (offer_ip >> 8) & 0xFF; opt->data[3] = (offer_ip >> 0) & 0xFF; opt = (struct dhcp_option *)((uint8_t *)opt + 6); + opt->code = DHCP_OPTION_LEASE_TIME; /* mandatory in a DHCPACK */ + opt->len = 4; + opt->data[0] = 0; opt->data[1] = 0; opt->data[2] = 0; opt->data[3] = 120; + opt = (struct dhcp_option *)((uint8_t *)opt + 6); opt->code = DHCP_OPTION_END; opt->len = 0; - ck_assert_int_eq(dhcp_parse_ack(&s, &msg, DHCP_HEADER_LEN + 26), 0); + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, DHCP_HEADER_LEN + 32), 0); ck_assert_uint_eq(primary->ip, offer_ip); ck_assert_uint_eq(primary->mask, mask); ck_assert_uint_eq(s.dhcp_server_ip, server_ip); @@ -782,10 +786,14 @@ START_TEST(test_dhcp_parse_ack_ignores_zero_len_unknown_option) opt->data[2] = (offer_ip >> 8) & 0xFF; opt->data[3] = (offer_ip >> 0) & 0xFF; opt = (struct dhcp_option *)((uint8_t *)opt + 6); + opt->code = DHCP_OPTION_LEASE_TIME; /* mandatory in a DHCPACK */ + opt->len = 4; + opt->data[0] = 0; opt->data[1] = 0; opt->data[2] = 0; opt->data[3] = 120; + opt = (struct dhcp_option *)((uint8_t *)opt + 6); opt->code = DHCP_OPTION_END; opt->len = 0; - ck_assert_int_eq(dhcp_parse_ack(&s, &msg, DHCP_HEADER_LEN + 25), 0); + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, DHCP_HEADER_LEN + 31), 0); ck_assert_uint_eq(primary->ip, offer_ip); ck_assert_uint_eq(primary->mask, mask); ck_assert_uint_eq(s.dhcp_server_ip, server_ip); diff --git a/src/wolfip.c b/src/wolfip.c index 0b2734e1..8b07489c 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -7799,7 +7799,12 @@ static int dhcp_parse_ack(struct wolfIP *s, struct dhcp_msg *msg, uint32_t msg_l } if (!saw_end) return -1; - if (primary && saw_server_id && + /* RFC 2131: the IP-address-lease-time option (51) is mandatory + * in a DHCPACK. lease_s is only ever set by that option (and a + * short option already returns -1 above), so lease_s != 0 means + * it was present with a valid nonzero duration. Without it the + * lease would be bound with no expiry/renewal timer. */ + if (primary && saw_server_id && lease_s != 0 && (primary->ip != 0) && (primary->mask != 0)) { dhcp_cancel_timer(s); s->dhcp_state = DHCP_BOUND; From 0c050fa3559d8a52cd5366cc21c342ee2f077172 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 16:41:08 +0200 Subject: [PATCH 14/24] F-5485: null-check the public DHCP helper APIs dhcp_bound(), dhcp_client_is_running() and dhcp_client_init() are public (in wolfip.h) but dereferenced the stack pointer without validating it, so a NULL argument crashed. Other public entry points in the stack already guard against NULL; these three were inconsistent. Return a deterministic value on NULL, matching the existing convention: the two predicates report 0 (not bound / not running) and dhcp_client_init() returns -WOLFIP_EINVAL. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_dhcp_edges.c | 10 ++++++++++ src/wolfip.c | 6 ++++++ 3 files changed, 17 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index fa891727..a2d85933 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -1344,6 +1344,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_dhcp_timer_cb_null_arg_noop); tcase_add_test(tc_core, test_dhcp_renew_rerandomizes_xid_rejecting_stale_ack); tcase_add_test(tc_core, test_dhcp_parse_ack_without_lease_time_rejected); + tcase_add_test(tc_core, test_dhcp_public_apis_null_stack_safe); /* --- unit_tests_ip_arp_recv.c (34 tests) --- */ tcase_add_test(tc_core, test_ip_recv_limited_broadcast_dst_is_local); tcase_add_test(tc_core, test_ip_recv_directed_broadcast_dst_is_local); diff --git a/src/test/unit/unit_tests_dhcp_edges.c b/src/test/unit/unit_tests_dhcp_edges.c index 087677c5..f167bb93 100644 --- a/src/test/unit/unit_tests_dhcp_edges.c +++ b/src/test/unit/unit_tests_dhcp_edges.c @@ -1238,3 +1238,13 @@ START_TEST(test_dhcp_parse_ack_without_lease_time_rejected) ck_assert_int_ne(s.dhcp_timer, NO_TIMER); } END_TEST + +/* F-5485: the public DHCP helper APIs must tolerate a NULL stack pointer and + * return a deterministic value instead of dereferencing it. */ +START_TEST(test_dhcp_public_apis_null_stack_safe) +{ + ck_assert_int_eq(dhcp_bound(NULL), 0); + ck_assert_int_eq(dhcp_client_is_running(NULL), 0); + ck_assert_int_eq(dhcp_client_init(NULL), -WOLFIP_EINVAL); +} +END_TEST diff --git a/src/wolfip.c b/src/wolfip.c index 8b07489c..53b374ba 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -8035,6 +8035,8 @@ static int dhcp_send_discover(struct wolfIP *s) int dhcp_bound(struct wolfIP *s) { + if (!s) + return 0; return (s->dhcp_state == DHCP_BOUND || s->dhcp_state == DHCP_RENEWING || s->dhcp_state == DHCP_REBINDING); @@ -8042,12 +8044,16 @@ int dhcp_bound(struct wolfIP *s) int dhcp_client_is_running(struct wolfIP *s) { + if (!s) + return 0; return DHCP_IS_RUNNING(s); } int dhcp_client_init(struct wolfIP *s) { struct wolfIP_sockaddr_in sin; + if (!s) + return -WOLFIP_EINVAL; if (s->dhcp_state != DHCP_OFF) return -1; s->dhcp_xid = wolfIP_getrandom(); From 5d62a3e1f0fe9e2b69093fe7e3ff6ab437d7dfd6 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 16:55:55 +0200 Subject: [PATCH 15/24] F-5733: validate ECHO_REPLY destination IP before delivering to ICMP sockets icmp_input dispatched ICMP_ECHO_REPLY straight to icmp_try_recv without the wolfIP_if_for_local_ip() guard the sibling ECHO_REQUEST path applies. With forwarding disabled there is no general "is this dst ours" gate before local dispatch, and icmp_try_recv skips the per-socket destination check when the socket's local_ip is 0 (reachable during/after DHCP) and ignores the ingress interface. An L2-adjacent attacker could thus address an echo reply to our MAC with an arbitrary ip.dst and a guessed 16-bit echo id and have it delivered to an application ICMP socket (fake ping-reply / covert-channel injection). Mirror the ECHO_REQUEST guard onto the reply branch: drop broadcast/multicast destinations and any reply whose ip.dst is not one of our configured local IPs. This bounds delivery (including to wildcard local_ip==0 sockets) to replies actually addressed to us, matching the RFC 1122 s3.2.2.6 policy already applied to requests, TCP and UDP. An existing test bound an ICMP socket to 10.0.0.1 without configuring that IP on an interface; it now also configures the interface so it keeps exercising reply delivery under the destination check. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_dns_dhcp.c | 53 +++++++++++++++++++++++++++++ src/wolfip.c | 14 ++++++++ 3 files changed, 68 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index a2d85933..a31969c6 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -850,6 +850,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_sock_connect_selects_local_ip_multi_if); tcase_add_test(tc_proto, test_icmp_socket_send_recv); tcase_add_test(tc_proto, test_icmp_input_echo_reply_queues); + tcase_add_test(tc_proto, test_icmp_input_echo_reply_wrong_dst_dropped); tcase_add_test(tc_proto, test_icmp_input_echo_request_reply_sent); tcase_add_test(tc_proto, test_icmp_input_echo_reply_sets_df); tcase_add_test(tc_proto, test_icmp_input_echo_request_bad_checksum_dropped); diff --git a/src/test/unit/unit_tests_dns_dhcp.c b/src/test/unit/unit_tests_dns_dhcp.c index b2417569..f0034bf3 100644 --- a/src/test/unit/unit_tests_dns_dhcp.c +++ b/src/test/unit/unit_tests_dns_dhcp.c @@ -1751,12 +1751,65 @@ START_TEST(test_icmp_input_echo_reply_queues) icmp.csum = ee16(icmp_checksum(&icmp, ICMP_HEADER_LEN)); frame_len = (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + ICMP_HEADER_LEN); + /* The reply is destined to a configured local IP (10.0.0.1). */ + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); icmp_input(&s, TEST_PRIMARY_IF, (struct wolfIP_ip_packet *)&icmp, frame_len); ck_assert_ptr_nonnull(fifo_peek(&ts->sock.udp.rxbuf)); ck_assert_uint_eq(ts->last_pkt_ttl, 55); } END_TEST +/* F-5733: icmp_input must only deliver an ECHO_REPLY that is actually addressed + * to one of our configured local IPs, mirroring the ECHO_REQUEST guard. A + * forged reply to a non-local dst (matching only a wildcard local_ip==0 socket + * by a guessed echo id) must be dropped. */ +START_TEST(test_icmp_input_echo_reply_wrong_dst_dropped) +{ + struct wolfIP s; + int icmp_sd; + struct tsocket *ts; + struct wolfIP_icmp_packet icmp; + uint32_t frame_len; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + icmp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_ICMP); + ck_assert_int_gt(icmp_sd, 0); + ts = &s.icmpsockets[SOCKET_UNMARK(icmp_sd)]; + ts->local_ip = 0; /* wildcard: per-socket dst check skipped */ + ts->remote_ip = 0; /* accept from any source */ + ts->src_port = ee16(0x1234); /* echo id the attacker guesses */ + frame_len = (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + ICMP_HEADER_LEN); + + /* Forged reply addressed to an IP that is NOT one of ours: must be dropped + * before reaching the socket. */ + memset(&icmp, 0, sizeof(icmp)); + icmp.ip.src = ee32(0x0A000002U); + icmp.ip.dst = ee32(0x0A000099U); /* not a configured local IP */ + icmp.ip.ttl = 55; + icmp.ip.len = ee16(IP_HEADER_LEN + ICMP_HEADER_LEN); + icmp.type = ICMP_ECHO_REPLY; + icmp_set_echo_id(&icmp, ts->src_port); + icmp.csum = ee16(icmp_checksum(&icmp, ICMP_HEADER_LEN)); + icmp_input(&s, TEST_PRIMARY_IF, (struct wolfIP_ip_packet *)&icmp, frame_len); + ck_assert_ptr_eq(fifo_peek(&ts->sock.udp.rxbuf), NULL); + + /* The same reply addressed to our configured local IP is delivered. */ + memset(&icmp, 0, sizeof(icmp)); + icmp.ip.src = ee32(0x0A000002U); + icmp.ip.dst = ee32(0x0A000001U); /* configured local IP */ + icmp.ip.ttl = 55; + icmp.ip.len = ee16(IP_HEADER_LEN + ICMP_HEADER_LEN); + icmp.type = ICMP_ECHO_REPLY; + icmp_set_echo_id(&icmp, ts->src_port); + icmp.csum = ee16(icmp_checksum(&icmp, ICMP_HEADER_LEN)); + icmp_input(&s, TEST_PRIMARY_IF, (struct wolfIP_ip_packet *)&icmp, frame_len); + ck_assert_ptr_nonnull(fifo_peek(&ts->sock.udp.rxbuf)); +} +END_TEST + START_TEST(test_icmp_input_echo_request_reply_sent) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 53b374ba..4e0598d0 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -7269,6 +7269,20 @@ static void icmp_input(struct wolfIP *s, unsigned int if_idx, struct wolfIP_ip_p if (wolfIP_filter_notify_icmp(WOLFIP_FILT_RECEIVING, s, if_idx, icmp, len) != 0) return; if (icmp->type == ICMP_ECHO_REPLY) { + ip4 dst = ee32(ip->dst); + int dst_match = 0; + /* RFC 1122 §3.2.2.6: only accept an echo reply that is actually + * addressed to one of our configured local IPs - the same guard the + * ECHO_REQUEST path below applies. Without it, an L2-adjacent attacker + * can address a frame to our MAC with an arbitrary ip.dst and a guessed + * echo id and have a forged reply delivered to an application ICMP + * socket (icmp_try_recv skips the per-socket dst check when the socket's + * local_ip is 0, e.g. during/after DHCP). */ + if (wolfIP_ip_is_broadcast(s, dst) || wolfIP_ip_is_multicast(dst)) + return; + (void)wolfIP_if_for_local_ip(s, dst, &dst_match); + if (!dst_match) + return; icmp_try_recv(s, if_idx, icmp, len); return; } From 9d05b09b7806c50c8b064c8ecbe8d42a9441ecc3 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 17:00:34 +0200 Subject: [PATCH 16/24] F-5069: defer UDP/ICMP bind src_port commit until the filter approves The UDP and ICMP arms of wolfIP_sock_bind wrote ts->src_port with the requested port/echo-id before calling the WOLFIP_FILT_BINDING filter callback, then rolled it back if the callback rejected the bind. The TCP arm already defers the assignment to the success path. wolfIP_filter_dispatch holds wolfip_filter_lock across the callback and makes any re-entrant filter dispatch return 0 (allow), so a callback that re-enters the stack via wolfIP_poll would have an ingress UDP/ICMP datagram matching local_ip:src_port delivered to the socket receive FIFO without WOLFIP_FILT_RECEIVING inspection - even when the bind is ultimately rejected. Move the src_port assignment after wolfIP_filter_notify_socket_event returns 0 in both arms, matching the TCP arm. The socket is no longer matchable by udp_try_recv / icmp_try_recv while the bind is being vetted. (Exploitation needs CONFIG_IPFILTER and a re-entrant filter callback; no in-tree callback does this, but it closes the ordering gap for such integrations.) --- src/test/unit/unit.c | 2 + src/test/unit/unit_tests_branches.c | 94 +++++++++++++++++++++++++++++ src/wolfip.c | 11 +++- 3 files changed, 105 insertions(+), 2 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index a31969c6..a71c201e 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -972,6 +972,8 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_sock_bind_icmp_basic_and_rebind); tcase_add_test(tc_core, test_sock_bind_icmp_wrong_family); tcase_add_test(tc_core, test_sock_bind_filter_block_rolls_back); + tcase_add_test(tc_core, test_udp_bind_src_port_deferred_until_filter_approves); + tcase_add_test(tc_core, test_icmp_bind_src_port_deferred_until_filter_approves); tcase_add_test(tc_core, test_sendto_arg_validation); tcase_add_test(tc_core, test_sendto_udp_short_addrlen_and_zero_dest); tcase_add_test(tc_core, test_sendto_udp_auto_assigns_src_port); diff --git a/src/test/unit/unit_tests_branches.c b/src/test/unit/unit_tests_branches.c index ad684f72..897b628d 100644 --- a/src/test/unit/unit_tests_branches.c +++ b/src/test/unit/unit_tests_branches.c @@ -569,6 +569,100 @@ START_TEST(test_sock_bind_filter_block_rolls_back) } END_TEST +/* F-5069: the bind callback must not see a committed src_port. If src_port is + * written before WOLFIP_FILT_BINDING runs, a callback that re-enters the stack + * (wolfIP_poll) would have an ingress datagram delivered to the socket while + * the bind is still being vetted (and even if it is rejected). Capture the + * socket's src_port as visible during the callback to prove it is deferred. */ +static struct wolfIP *bind_toctou_stack; +static unsigned int bind_toctou_idx; +static int bind_toctou_is_icmp; +static uint16_t bind_toctou_port_during_cb; +static int bind_toctou_capture_cb(void *arg, const struct wolfIP_filter_event *event) +{ + (void)arg; + if (event && event->reason == WOLFIP_FILT_BINDING && bind_toctou_stack) { + bind_toctou_port_during_cb = bind_toctou_is_icmp ? + bind_toctou_stack->icmpsockets[bind_toctou_idx].src_port : + bind_toctou_stack->udpsockets[bind_toctou_idx].src_port; + } + return 1; /* reject the bind */ +} + +START_TEST(test_udp_bind_src_port_deferred_until_filter_approves) +{ + struct wolfIP s; + int udp_sd; + struct tsocket *ts; + struct wolfIP_sockaddr_in sin; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); + ck_assert_int_gt(udp_sd, 0); + ts = &s.udpsockets[SOCKET_UNMARK(udp_sd)]; + + bind_toctou_stack = &s; + bind_toctou_idx = SOCKET_UNMARK(udp_sd); + bind_toctou_is_icmp = 0; + bind_toctou_port_during_cb = 0xFFFF; + wolfIP_filter_set_callback(bind_toctou_capture_cb, NULL); + wolfIP_filter_set_mask(WOLFIP_FILT_MASK(WOLFIP_FILT_BINDING)); + + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_port = ee16(4444); + sin.sin_addr.s_addr = ee32(0x0A000001U); + ck_assert_int_eq(wolfIP_sock_bind(&s, udp_sd, + (struct wolfIP_sockaddr *)&sin, sizeof(sin)), -1); + + wolfIP_filter_set_callback(NULL, NULL); + wolfIP_filter_set_mask(0); + + /* The port was not committed (matchable) while the filter was deciding. */ + ck_assert_uint_eq(bind_toctou_port_during_cb, 0); + /* A rejected bind leaves the socket unbound. */ + ck_assert_uint_eq(ts->src_port, 0); +} +END_TEST + +START_TEST(test_icmp_bind_src_port_deferred_until_filter_approves) +{ + struct wolfIP s; + int icmp_sd; + struct tsocket *ts; + struct wolfIP_sockaddr_in sin; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + icmp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_ICMP); + ck_assert_int_gt(icmp_sd, 0); + ts = &s.icmpsockets[SOCKET_UNMARK(icmp_sd)]; + + bind_toctou_stack = &s; + bind_toctou_idx = SOCKET_UNMARK(icmp_sd); + bind_toctou_is_icmp = 1; + bind_toctou_port_during_cb = 0xFFFF; + wolfIP_filter_set_callback(bind_toctou_capture_cb, NULL); + wolfIP_filter_set_mask(WOLFIP_FILT_MASK(WOLFIP_FILT_BINDING)); + + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_port = ee16(0x4321); /* ICMP echo id */ + sin.sin_addr.s_addr = ee32(0x0A000001U); + ck_assert_int_eq(wolfIP_sock_bind(&s, icmp_sd, + (struct wolfIP_sockaddr *)&sin, sizeof(sin)), -1); + + wolfIP_filter_set_callback(NULL, NULL); + wolfIP_filter_set_mask(0); + + ck_assert_uint_eq(bind_toctou_port_during_cb, 0); + ck_assert_uint_eq(ts->src_port, 0); +} +END_TEST + /* ---- wolfIP_sock_sendto extras ---- */ START_TEST(test_sendto_arg_validation) diff --git a/src/wolfip.c b/src/wolfip.c index 4e0598d0..99f64f06 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -7106,7 +7106,11 @@ int wolfIP_sock_bind(struct wolfIP *s, int sockfd, const struct wolfIP_sockaddr ts->local_ip = prev_ip; return -1; } - ts->src_port = new_port; + /* Commit src_port only after the filter approves the bind (as the + * TCP arm does): otherwise the socket is matchable by the ingress + * path while the BINDING callback runs, and a callback that + * re-enters wolfIP_poll would get a datagram delivered to it even + * if the bind is ultimately rejected. */ if (wolfIP_filter_notify_socket_event( WOLFIP_FILT_BINDING, s, ts, ts->local_ip, new_port, IPADDR_ANY, 0) != 0) { @@ -7114,6 +7118,7 @@ int wolfIP_sock_bind(struct wolfIP *s, int sockfd, const struct wolfIP_sockaddr ts->src_port = prev_port; return -1; } + ts->src_port = new_port; } ts->bound_local_ip = bind_ip; return 0; @@ -7144,7 +7149,8 @@ int wolfIP_sock_bind(struct wolfIP *s, int sockfd, const struct wolfIP_sockaddr ts->local_ip = prev_ip; return -1; } - ts->src_port = new_id; + /* Commit the echo id only after the filter approves (see the UDP + * arm): keep the socket unmatchable by icmp_try_recv until then. */ if (wolfIP_filter_notify_socket_event( WOLFIP_FILT_BINDING, s, ts, ts->local_ip, new_id, IPADDR_ANY, 0) != 0) { @@ -7152,6 +7158,7 @@ int wolfIP_sock_bind(struct wolfIP *s, int sockfd, const struct wolfIP_sockaddr ts->src_port = prev_id; return -1; } + ts->src_port = new_id; } ts->bound_local_ip = bind_ip; return 0; From 57e8affee611a213508bf02fc40101c032ffef20 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 17:03:37 +0200 Subject: [PATCH 17/24] F-5495: reject a NULL receive buffer in wolfIP_sock_recvfrom wolfIP_sock_recvfrom validated sockfd and s but never buf, then copied queued data into it for every socket family (TCP via queue_pop, UDP/ICMP/raw/packet via memcpy). A caller passing buf == NULL with a nonzero length, once a datagram had queued, dereferenced NULL and crashed. Reject a NULL buffer with nonzero length up front (before any family branch), returning -WOLFIP_EINVAL and consuming nothing. len == 0 (a no-op data-availability probe) remains allowed. This guards TCP, UDP, ICMP, raw and packet sockets uniformly, matching the existing sockfd/s validation. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_dns_dhcp.c | 29 +++++++++++++++++++++++++++++ src/wolfip.c | 7 +++++++ 3 files changed, 37 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index a71c201e..2b4acac9 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -202,6 +202,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_sock_recvfrom_short_addrlen); tcase_add_test(tc_utils, test_sock_recvfrom_udp_short_addrlen); tcase_add_test(tc_utils, test_sock_recvfrom_icmp_short_addrlen); + tcase_add_test(tc_utils, test_sock_recvfrom_null_buf_rejected); tcase_add_test(tc_utils, test_sock_recvfrom_udp_fifo_alignment); tcase_add_test(tc_utils, test_sock_recvfrom_tcp_close_wait_sets_readable); tcase_add_test(tc_utils, test_sock_recvfrom_udp_readable_stays_when_queue_nonempty); diff --git a/src/test/unit/unit_tests_dns_dhcp.c b/src/test/unit/unit_tests_dns_dhcp.c index f0034bf3..75abea98 100644 --- a/src/test/unit/unit_tests_dns_dhcp.c +++ b/src/test/unit/unit_tests_dns_dhcp.c @@ -1588,6 +1588,35 @@ START_TEST(test_sock_recvfrom_icmp_short_addrlen) } END_TEST +/* F-5495: a NULL receive buffer with a nonzero length must be rejected instead + * of being copied into. With a datagram already queued, the prior code reached + * memcpy(buf=NULL, ...) and crashed. */ +START_TEST(test_sock_recvfrom_null_buf_rejected) +{ + struct wolfIP s; + int udp_sd; + struct tsocket *ts; + uint8_t payload[4] = { 1, 2, 3, 4 }; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); + ck_assert_int_gt(udp_sd, 0); + ts = &s.udpsockets[SOCKET_UNMARK(udp_sd)]; + ts->src_port = ee16(4444); + + /* Queue a datagram so the recvfrom path would otherwise reach the copy. */ + enqueue_udp_rx(ts, payload, sizeof(payload), 5555); + + ck_assert_int_eq(wolfIP_sock_recvfrom(&s, udp_sd, NULL, 64, 0, NULL, NULL), + -WOLFIP_EINVAL); + /* Rejected before any dequeue: the datagram is still queued. */ + ck_assert_uint_gt(fifo_len(&ts->sock.udp.rxbuf), 0U); +} +END_TEST + START_TEST(test_sock_recvfrom_udp_fifo_alignment) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 99f64f06..ceea9d5b 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -6149,6 +6149,13 @@ int wolfIP_sock_recvfrom(struct wolfIP *s, int sockfd, void *buf, size_t len, in if (!s) return -WOLFIP_EINVAL; + /* A nonzero-length read needs a destination buffer: every socket family + * below copies queued data into buf (queue_pop / memcpy). Reject a NULL + * buffer here so caller misuse cannot dereference it. len == 0 (a probe) + * stays allowed. */ + if (!buf && len > 0) + return -WOLFIP_EINVAL; + if (IS_SOCKET_TCP(sockfd)) { if (SOCKET_UNMARK(sockfd) >= MAX_TCPSOCKETS) return -WOLFIP_EINVAL; From eae67c174341e1a924d20e61db4f4607524479a3 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 17:06:37 +0200 Subject: [PATCH 18/24] F-5479: validate TCP connect local binding before mutating socket state The TCP arm of wolfIP_sock_connect set ts->sock.tcp.state = TCP_SYN_SENT and ts->remote_ip before validating that bound_local_ip still resolves to an interface. When the bound address had been removed/changed (via wolfIP_ipconfig_set_ex), the bound_match check returned -WOLFIP_EINVAL with the socket left in TCP_SYN_SENT - but no SYN was queued and no RTO timer started. Every later connect then hit the "state == TCP_SYN_SENT -> return EAGAIN" early path, wedging the socket permanently. The other connect error paths (filter reject, tcp_send_syn failure) already roll state back to CLOSED; only the binding-validation path did not. Resolve and validate the binding into locals first and only then commit state/remote_ip/if_idx/local_ip, mirroring the UDP arm. A failed validation now returns EINVAL with the socket still CLOSED, so a retry re-validates instead of returning a phantom in-flight SYN. The existing test only checked the return code; it now also asserts the socket stays CLOSED and that a second connect re-validates (EINVAL, not stuck EAGAIN). --- src/test/unit/unit_tests_api.c | 8 ++++++++ src/wolfip.c | 28 +++++++++++++++++++--------- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/test/unit/unit_tests_api.c b/src/test/unit/unit_tests_api.c index 9753e167..360c9d18 100644 --- a/src/test/unit/unit_tests_api.c +++ b/src/test/unit/unit_tests_api.c @@ -1957,6 +1957,14 @@ START_TEST(test_sock_connect_tcp_bound_local_ip_no_match) sin.sin_addr.s_addr = ee32(0x0A000002U); ck_assert_int_eq(wolfIP_sock_connect(&s, tcp_sd, (struct wolfIP_sockaddr *)&sin, sizeof(sin)), -WOLFIP_EINVAL); + /* F-5479: a rejected connect must leave the socket CLOSED, not wedged in + * SYN_SENT. Otherwise the next connect returns a stuck EAGAIN for a SYN + * that was never queued. */ + ck_assert_int_eq(ts->sock.tcp.state, TCP_CLOSED); + /* Retrying must re-validate (EINVAL again), not report a phantom in-flight + * SYN. */ + ck_assert_int_eq(wolfIP_sock_connect(&s, tcp_sd, (struct wolfIP_sockaddr *)&sin, sizeof(sin)), -WOLFIP_EINVAL); + ck_assert_int_eq(ts->sock.tcp.state, TCP_CLOSED); } END_TEST diff --git a/src/wolfip.c b/src/wolfip.c index ceea9d5b..0b77d7c0 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -5602,29 +5602,39 @@ int wolfIP_sock_connect(struct wolfIP *s, int sockfd, const struct wolfIP_sockad return -WOLFIP_EINVAL; if (ts->sock.tcp.state == TCP_CLOSED) { struct ipconf *conf; - ts->sock.tcp.state = TCP_SYN_SENT; - ts->remote_ip = ee32(sin->sin_addr.s_addr); + ip4 new_remote_ip = ee32(sin->sin_addr.s_addr); + uint8_t new_if_idx; + ip4 new_local_ip; + + /* Resolve and validate the local binding into locals before mutating + * the socket. A failed validation here must not leave the socket in + * TCP_SYN_SENT (no SYN queued, no RTO timer), which would make every + * later connect return EAGAIN forever. Mirrors the UDP arm above. */ if (ts->bound_local_ip != IPADDR_ANY) { int bound_match = 0; unsigned int bound_if = wolfIP_if_for_local_ip(s, ts->bound_local_ip, &bound_match); if (!bound_match) return -WOLFIP_EINVAL; - ts->if_idx = (uint8_t)bound_if; - ts->local_ip = ts->bound_local_ip; + new_if_idx = (uint8_t)bound_if; + new_local_ip = ts->bound_local_ip; } else { - if_idx = wolfIP_route_for_ip(s, ts->remote_ip); + if_idx = wolfIP_route_for_ip(s, new_remote_ip); conf = wolfIP_ipconf_at(s, if_idx); - ts->if_idx = (uint8_t)if_idx; + new_if_idx = (uint8_t)if_idx; if (conf && conf->ip != IPADDR_ANY) - ts->local_ip = conf->ip; + new_local_ip = conf->ip; else { struct ipconf *primary = wolfIP_primary_ipconf(s); if (primary && primary->ip != IPADDR_ANY) - ts->local_ip = primary->ip; + new_local_ip = primary->ip; else - ts->local_ip = IPADDR_ANY; + new_local_ip = IPADDR_ANY; } } + ts->sock.tcp.state = TCP_SYN_SENT; + ts->remote_ip = new_remote_ip; + ts->if_idx = new_if_idx; + ts->local_ip = new_local_ip; if (!ts->src_port) ts->src_port = (uint16_t)(wolfIP_getrandom() & 0xFFFF); if (ts->src_port < 1024) From 99cbd86600e7e2e5006212562171d67c001cf89e Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 17:12:10 +0200 Subject: [PATCH 19/24] F-5904: validate IGMP query TTL and destination in igmp_input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit igmp_input accepted any IGMP membership query that passed the length, checksum, type and group-is-multicast checks, ignoring ip->ttl and ip->dst. ip_recv's source filter only drops broadcast/multicast/zero sources, so an off-link or unicast-addressed query (e.g. TTL=64, dst=our unicast IP) reached igmp_input and solicited an IGMPv3 membership report per joined group - disclosing the host's multicast membership table and violating RFC 3376 §4.1. Add the RFC 3376 §4.1 query guards: drop the query unless ip->ttl == 1 (IGMP is link-local and never transits a router) and its destination is either all-hosts (224.0.0.1) for a general query or the queried group for a group-specific query. Both fields are already reachable via ip; no new parsing or helper is needed, and legitimate on-link queries (TTL 1, dst 224.0.0.1/group) are unaffected. Note: the report also suggested an on-link source-address check; that needs a new subnet helper and is largely redundant with the TTL==1 guard (a TTL==1 query cannot have transited a router), so it is not added here. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_multicast.c | 88 ++++++++++++++++++++++++++++ src/wolfip.c | 14 +++++ 3 files changed, 103 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 2b4acac9..49fef055 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -273,6 +273,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_multicast_udp_send_mac_ttl_loop_and_options); tcase_add_test(tc_utils, test_multicast_igmp_query_refreshes_report); tcase_add_test(tc_utils, test_multicast_igmp_query_bad_checksum_dropped); + tcase_add_test(tc_utils, test_multicast_igmp_query_spoofed_dropped); tcase_add_test(tc_utils, test_multicast_join_requires_configured_ip); tcase_add_test(tc_utils, test_multicast_if_pins_egress_interface); tcase_add_test(tc_utils, test_multicast_loop_does_not_fire_on_blocked_send); diff --git a/src/test/unit/unit_tests_multicast.c b/src/test/unit/unit_tests_multicast.c index 9c93096a..72886c3c 100644 --- a/src/test/unit/unit_tests_multicast.c +++ b/src/test/unit/unit_tests_multicast.c @@ -314,6 +314,94 @@ START_TEST(test_multicast_igmp_query_bad_checksum_dropped) } END_TEST +/* F-5904: igmp_input must apply RFC 3376 §4.1 query validation. A query that + * could not be a legitimate on-link membership query - TTL != 1 (transited a + * router), or a destination that is neither all-hosts (224.0.0.1) nor the + * group - must not solicit membership reports (which would disclose the host's + * group memberships). */ +START_TEST(test_multicast_igmp_query_spoofed_dropped) +{ + struct wolfIP s; + int sd; + struct wolfIP_ip_mreq mreq; + struct wolfIP_ll_dev *ll; + uint8_t frame[ETH_HEADER_LEN + IP_HEADER_LEN + IGMPV3_QUERY_MIN_LEN]; + struct wolfIP_ip_packet *ip = (struct wolfIP_ip_packet *)frame; + uint8_t *igmp = frame + ETH_HEADER_LEN + IP_HEADER_LEN; + ip4 group = 0xE9010207U; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000002U, 0xFFFFFF00U, 0); + ll = wolfIP_getdev_ex(&s, TEST_PRIMARY_IF); + ck_assert_ptr_nonnull(ll); + sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); + ck_assert_int_gt(sd, 0); + multicast_mreq(&mreq, group, IPADDR_ANY); + ck_assert_int_eq(wolfIP_sock_setsockopt(&s, sd, WOLFIP_SOL_IP, + WOLFIP_IP_ADD_MEMBERSHIP, &mreq, sizeof(mreq)), 0); + + /* (1) Otherwise-valid general query but with TTL != 1 -> dropped. */ + memset(frame, 0, sizeof(frame)); + memcpy(ip->eth.dst, "\x01\x00\x5e\x00\x00\x01", 6); + memcpy(ip->eth.src, "\x02\x00\x00\x00\x00\x01", 6); + ip->eth.type = ee16(ETH_TYPE_IP); + ip->ver_ihl = 0x45; + ip->ttl = 64; + ip->proto = WI_IPPROTO_IGMP; + ip->len = ee16(IP_HEADER_LEN + IGMPV3_QUERY_MIN_LEN); + ip->src = ee32(0x0A000001U); + ip->dst = ee32(IGMP_ALL_HOSTS); + igmp[0] = IGMP_TYPE_MEMBERSHIP_QUERY; + put_be32(igmp + 4, group); + put_be16(igmp + 2, ip_checksum_buf(igmp, IGMPV3_QUERY_MIN_LEN)); + fix_ip_checksum(ip); + last_frame_sent_size = 0; + wolfIP_recv_ex(&s, TEST_PRIMARY_IF, frame, sizeof(frame)); + ck_assert_uint_eq(last_frame_sent_size, 0); + + /* (2) TTL == 1 but addressed to our unicast IP (not all-hosts/group) -> + * dropped. Sent to our unicast MAC so it reaches igmp_input. */ + memset(frame, 0, sizeof(frame)); + memcpy(ip->eth.dst, ll->mac, 6); + memcpy(ip->eth.src, "\x02\x00\x00\x00\x00\x01", 6); + ip->eth.type = ee16(ETH_TYPE_IP); + ip->ver_ihl = 0x45; + ip->ttl = 1; + ip->proto = WI_IPPROTO_IGMP; + ip->len = ee16(IP_HEADER_LEN + IGMPV3_QUERY_MIN_LEN); + ip->src = ee32(0x0A000001U); + ip->dst = ee32(0x0A000002U); /* our unicast IP */ + igmp[0] = IGMP_TYPE_MEMBERSHIP_QUERY; + put_be32(igmp + 4, group); + put_be16(igmp + 2, ip_checksum_buf(igmp, IGMPV3_QUERY_MIN_LEN)); + fix_ip_checksum(ip); + last_frame_sent_size = 0; + wolfIP_recv_ex(&s, TEST_PRIMARY_IF, frame, sizeof(frame)); + ck_assert_uint_eq(last_frame_sent_size, 0); + + /* Sanity: a compliant query (TTL 1, all-hosts dst) still solicits a report, + * so the guards did not over-block. */ + memset(frame, 0, sizeof(frame)); + memcpy(ip->eth.dst, "\x01\x00\x5e\x00\x00\x01", 6); + memcpy(ip->eth.src, "\x02\x00\x00\x00\x00\x01", 6); + ip->eth.type = ee16(ETH_TYPE_IP); + ip->ver_ihl = 0x45; + ip->ttl = 1; + ip->proto = WI_IPPROTO_IGMP; + ip->len = ee16(IP_HEADER_LEN + IGMPV3_QUERY_MIN_LEN); + ip->src = ee32(0x0A000001U); + ip->dst = ee32(IGMP_ALL_HOSTS); + igmp[0] = IGMP_TYPE_MEMBERSHIP_QUERY; + put_be32(igmp + 4, group); + put_be16(igmp + 2, ip_checksum_buf(igmp, IGMPV3_QUERY_MIN_LEN)); + fix_ip_checksum(ip); + last_frame_sent_size = 0; + wolfIP_recv_ex(&s, TEST_PRIMARY_IF, frame, sizeof(frame)); + ck_assert_uint_gt(last_frame_sent_size, 0); +} +END_TEST + START_TEST(test_multicast_join_requires_configured_ip) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 0b77d7c0..bc1c2ab9 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -4012,6 +4012,12 @@ static void igmp_input(struct wolfIP *s, unsigned int if_idx, return; if (igmp[0] != IGMP_TYPE_MEMBERSHIP_QUERY) return; + /* RFC 3376 §4.1.1 / RFC 2236 §2: IGMP messages carry IP TTL 1 and are + * link-local; a query with any other TTL transited a router and cannot be + * a legitimate on-link query. Dropping it stops an off-link/spoofed query + * from soliciting (and thereby disclosing) the host's membership reports. */ + if (ip->ttl != 1) + return; /* RFC 2236 §2 (IGMPv2) and RFC 3376 §4.1 (IGMPv3) both place the Group * Address at offset 4 within the message. Read unconditionally so that * IGMPv1/v2 group-specific queries (8-byte messages) are not silently @@ -4019,6 +4025,14 @@ static void igmp_input(struct wolfIP *s, unsigned int if_idx, group = get_be32(igmp + 4); if (group != IPADDR_ANY && !wolfIP_ip_is_multicast(group)) return; + /* RFC 3376 §4.1.2: a general query is addressed to all-hosts (224.0.0.1) + * and a group-specific query to the group itself. Reject a query sent to + * any other destination (e.g. our unicast address). */ + { + ip4 dst = ee32(ip->dst); + if (dst != IGMP_ALL_HOSTS && dst != group) + return; + } for (i = 0; i < WOLFIP_MCAST_MEMBERSHIPS; i++) { if (s->mcast[i].refs == 0 || s->mcast[i].if_idx != if_idx) From 58f791b3f5f39f7673c762995d90d348ab97fe7c Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 17:55:05 +0200 Subject: [PATCH 20/24] F-5490: make iphdr_set_checksum clear the csum field before computing iphdr_set_checksum summed the whole header including whatever was already in ip->csum, so it produced a correct value only if the caller had zeroed the field first. All current callers do (explicitly or via memset), so this was not a live bug, but the implicit precondition is a footgun: a future caller that forgets to clear csum would emit a silently-wrong checksum. Zero ip->csum inside the setter before summing (RFC 1071). The setter is now correct-by-construction and idempotent (set/verify holds for any input, set;set;verify holds); existing callers that already clear the field are unaffected (the extra clear is a no-op). --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_proto.c | 27 +++++++++++++++++++++++++++ src/wolfip.c | 5 +++++ 3 files changed, 33 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 49fef055..dd534a22 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -927,6 +927,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_transport_checksum); tcase_add_test(tc_utils, test_iphdr_set_checksum); + tcase_add_test(tc_utils, test_iphdr_set_checksum_idempotent_with_stale_csum); tcase_add_test(tc_utils, test_eth_output_add_header); tcase_add_test(tc_utils, test_eth_output_add_header_invalid_if); tcase_add_test(tc_utils, test_ip_output_add_header); diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index f009558c..404261ab 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -4512,6 +4512,33 @@ START_TEST(test_iphdr_set_checksum) { } END_TEST +/* F-5490: iphdr_set_checksum must produce a valid checksum regardless of any + * stale value already in ip->csum, and must be idempotent under repeated calls + * (the setter clears the field itself rather than relying on the caller). */ +START_TEST(test_iphdr_set_checksum_idempotent_with_stale_csum) { + struct wolfIP_ip_packet ip; + memset(&ip, 0, sizeof(ip)); + + ip.ver_ihl = 0x45; + ip.tos = 0; + ip.len = ee16(20); + ip.id = ee16(1); + ip.flags_fo = 0; + ip.ttl = 64; + ip.proto = WI_IPPROTO_TCP; + ip.src = ee32(0xc0a80101); + ip.dst = ee32(0xc0a80102); + ip.csum = ee16(0xBEEF); /* stale, nonzero checksum left by the caller */ + + iphdr_set_checksum(&ip); + ck_assert_int_eq(iphdr_verify_checksum(&ip), 0); + + /* Running it again over the now-populated header must still verify. */ + iphdr_set_checksum(&ip); + ck_assert_int_eq(iphdr_verify_checksum(&ip), 0); +} +END_TEST + // Test for `eth_output_add_header` to add Ethernet headers START_TEST(test_eth_output_add_header) { struct wolfIP_eth_frame eth_frame; diff --git a/src/wolfip.c b/src/wolfip.c index bc1c2ab9..def4e180 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -3859,6 +3859,11 @@ static void iphdr_set_checksum(struct wolfIP_ip_packet *ip) if (ip_hlen < IP_HEADER_LEN) ip_hlen = IP_HEADER_LEN; + /* Zero the checksum field before summing (RFC 1071) so the result is + * correct regardless of any stale value the caller left in ip->csum. This + * makes the setter idempotent (set/verify always holds) and removes the + * implicit "caller must clear csum first" precondition. */ + ip->csum = 0; for (i = 0; i < ip_hlen; i += 2) { uint16_t word; memcpy(&word, ptr + i, sizeof(word)); From 6e1f5840a0dd499e7baefa32b43c7f5fcc4c0b43 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 18:06:47 +0200 Subject: [PATCH 21/24] F-4949: coalesce overlapping OOO segments instead of consuming a slot each tcp_store_ooo_segment treated only an exact (seq,len) match as a duplicate, so segments with distinct (seq,len) pairs that cover overlapping byte ranges each took a separate slot. With only TCP_OOO_MAX_SEGS=4 slots, an in-window injector (or a peer re-segmenting retransmissions) could fill the cache with four overlapping segments, after which every further legitimate out-of-order segment was silently dropped, degrading goodput until the in-order hole was refilled. Replace the exact-match dedup with an overlap check that coalesces the incoming segment into the overlapping slot: store the union of the two ranges in place (incoming bytes win in the overlap) when it fits a single slot. This bounds how many slots an overlapping cluster can occupy. A simple replace-on-overlap would have lost the non-overlapping tail of the cached segment (breaking the existing coalesce-on-consume behaviour); the union merge preserves it. When the union would exceed one slot the segments are kept separate, as before, and tcp_consume_ooo coalesces them on promotion. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_tcp_ack.c | 38 ++++++++++++++++++++++++++++++ src/wolfip.c | 35 +++++++++++++++++++++++---- 3 files changed, 69 insertions(+), 5 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index dd534a22..d71deea9 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -650,6 +650,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_tcp_rebuild_rx_sack_triggering_block_first); tcase_add_test(tc_utils, test_tcp_consume_ooo_wrap_trim_and_promote); tcase_add_test(tc_utils, test_tcp_consume_ooo_wrap_drop_fully_acked); + tcase_add_test(tc_utils, test_tcp_store_ooo_overlap_does_not_exhaust_cache); tcase_add_test(tc_utils, test_tcp_ack_sack_early_retransmit_before_three_dupack); tcase_add_test(tc_utils, test_tcp_input_listen_syn_without_sack_disables_sack); tcase_add_test(tc_utils, test_tcp_input_listen_syn_arms_control_rto); diff --git a/src/test/unit/unit_tests_tcp_ack.c b/src/test/unit/unit_tests_tcp_ack.c index 7a545340..c70a77ab 100644 --- a/src/test/unit/unit_tests_tcp_ack.c +++ b/src/test/unit/unit_tests_tcp_ack.c @@ -4441,6 +4441,44 @@ START_TEST(test_tcp_consume_ooo_wrap_drop_fully_acked) } END_TEST +/* F-4949: overlapping out-of-order segments with distinct (seq,len) must not + * each consume a slot, or an attacker (or a peer re-segmenting retransmissions) + * could exhaust the OOO cache and get all later legitimate OOO data dropped. */ +START_TEST(test_tcp_store_ooo_overlap_does_not_exhaust_cache) +{ + struct wolfIP s; + struct tsocket *ts; + uint8_t payload[100] = {0}; + int i, used; + + wolfIP_init(&s); + ts = &s.tcpsockets[0]; + memset(ts, 0, sizeof(*ts)); + ts->proto = WI_IPPROTO_TCP; + ts->S = &s; + ts->sock.tcp.state = TCP_ESTABLISHED; + ts->sock.tcp.ack = 1000; + + /* Four overlapping segments covering essentially the same bytes. */ + ck_assert_int_eq(tcp_store_ooo_segment(ts, payload, 1100, 100), 0); + ck_assert_int_eq(tcp_store_ooo_segment(ts, payload, 1101, 100), 0); + ck_assert_int_eq(tcp_store_ooo_segment(ts, payload, 1200, 100), 0); + ck_assert_int_eq(tcp_store_ooo_segment(ts, payload, 1201, 100), 0); + + used = 0; + for (i = 0; i < TCP_OOO_MAX_SEGS; i++) + if (ts->sock.tcp.ooo[i].used) + used++; + /* The overlapping cluster must leave free slots for real OOO data. */ + ck_assert_int_lt(used, TCP_OOO_MAX_SEGS); + + /* Legitimate non-overlapping OOO segments are still accepted (not dropped + * by a cache exhausted with overlapping junk). */ + ck_assert_int_eq(tcp_store_ooo_segment(ts, payload, 2000, 100), 0); + ck_assert_int_eq(tcp_store_ooo_segment(ts, payload, 2200, 100), 0); +} +END_TEST + START_TEST(test_tcp_ack_sack_early_retransmit_before_three_dupack) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index def4e180..5f595f0e 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -3001,11 +3001,36 @@ static int tcp_store_ooo_segment(struct tsocket *t, const uint8_t *data, slot = (int)i; continue; } - /* Duplicate range: keep newest bytes and avoid consuming another slot. */ - if (t->sock.tcp.ooo[i].seq == seq && t->sock.tcp.ooo[i].len == len) { - memcpy(t->sock.tcp.ooo[i].data, data, len); - tcp_rebuild_rx_sack(t, seq, len); - return 0; + /* Overlapping range (including an exact duplicate): coalesce into this + * slot rather than consuming another. Otherwise an attacker injecting + * distinct (seq,len) pairs over the same bytes - or a peer that + * re-segments retransmissions - could occupy every slot with overlapping + * data and starve later legitimate OOO segments. The union of the two + * ranges is stored in place (incoming bytes win in the overlap) when it + * fits one slot; if it would not fit, fall through and keep them as + * separate slots (tcp_consume_ooo coalesces them on promotion). */ + { + uint32_t cur_seq = t->sock.tcp.ooo[i].seq; + uint32_t cur_len = t->sock.tcp.ooo[i].len; + uint32_t end_new = tcp_seq_inc(seq, len); + uint32_t end_cur = tcp_seq_inc(cur_seq, cur_len); + if (tcp_seq_lt(seq, end_cur) && tcp_seq_lt(cur_seq, end_new)) { + uint32_t u_start = tcp_seq_lt(seq, cur_seq) ? seq : cur_seq; + uint32_t u_end = tcp_seq_lt(end_cur, end_new) ? end_new : end_cur; + uint32_t u_len = (uint32_t)tcp_seq_diff(u_end, u_start); + if (u_len <= TCP_MSS_MAX) { + uint8_t *buf = t->sock.tcp.ooo[i].data; + uint32_t cur_off = (uint32_t)tcp_seq_diff(cur_seq, u_start); + uint32_t new_off = (uint32_t)tcp_seq_diff(seq, u_start); + if (cur_off != 0) + memmove(buf + cur_off, buf, cur_len); + memcpy(buf + new_off, data, len); + t->sock.tcp.ooo[i].seq = u_start; + t->sock.tcp.ooo[i].len = u_len; + tcp_rebuild_rx_sack(t, u_start, u_len); + return 0; + } + } } } if (slot < 0) From bf2c6250f9ce1f88951e3e200b850aead1d56829 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 18:11:33 +0200 Subject: [PATCH 22/24] F-5488: reject overlong TCP Timestamp options instead of accepting them The TS branch in tcp_parse_options accepted any option with olen >= TCP_OPTION_TS_LEN, so a SYN carrying kind 8 with length 11-40 still set ts_found, negotiated ts_enabled, and recorded TSval/TSEcr. RFC 7323 fixes the Timestamp option at length 10, and every other fixed-length option here (WS, MSS, SACK-permitted) already enforces an exact olen; only TS used >=. There is no memory-safety issue (the 8 read bytes are within the already-validated olen bounds), but accepting a non-canonical length violates the encode/decode roundtrip property. Require olen == TCP_OPTION_TS_LEN. When the length is wrong the branch is now skipped and opt += olen (already bounds-checked) advances parsing correctly, so the remaining options are unaffected. Test test_tcp_parse_options_timestamp_overlong_ignored injects a SYN with an olen=11 TS option and asserts ts_enabled stays 0; it fails pre-fix (ts_enabled==1) and passes after. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_tcp_state.c | 39 ++++++++++++++++++++++++++++ src/wolfip.c | 2 +- 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index d71deea9..1148d067 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -1187,6 +1187,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_tcp_parse_options_nop_advances); tcase_add_test(tc_core, test_tcp_parse_options_zero_olen_breaks); tcase_add_test(tc_core, test_tcp_parse_options_timestamp_parsed); + tcase_add_test(tc_core, test_tcp_parse_options_timestamp_overlong_ignored); tcase_add_test(tc_core, test_tcp_parse_options_mss_zero_ignored); tcase_add_test(tc_core, test_tcp_parse_options_sack_permitted_parsed); tcase_add_test(tc_core, test_tcp_input_syn_rcvd_rst_bad_seq_ignored); diff --git a/src/test/unit/unit_tests_tcp_state.c b/src/test/unit/unit_tests_tcp_state.c index 7be2fc2e..bf273b47 100644 --- a/src/test/unit/unit_tests_tcp_state.c +++ b/src/test/unit/unit_tests_tcp_state.c @@ -396,6 +396,45 @@ START_TEST(test_tcp_parse_options_timestamp_parsed) } END_TEST +/* Overlong Timestamp option (olen > canonical 10) must be rejected. + * RFC 7323 fixes the TS option at length 10; only canonical lengths + * should roundtrip. An olen=11 TS must not negotiate ts_enabled. */ +START_TEST(test_tcp_parse_options_timestamp_overlong_ignored) +{ + /* TS option with olen=11 (one byte longer than canonical). The 9 + * trailing bytes still carry a well-formed TSval/TSEcr; only the + * length is wrong. */ + uint8_t opts[] = { + TCP_OPTION_TS, 11, + 0x00, 0x00, 0x01, 0x02, /* TSval = 0x0102 */ + 0x00, 0x00, 0x00, 0x00, /* TSEcr = 0 */ + 0x00, /* extra byte: olen=11, not 10 */ + TCP_OPTION_EOO + }; + struct wolfIP s; + struct tsocket *ts; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + ts = &s.tcpsockets[0]; + memset(ts, 0, sizeof(*ts)); + ts->proto = WI_IPPROTO_TCP; + ts->S = &s; + ts->sock.tcp.state = TCP_LISTEN; + ts->src_port = 8080; + ts->sock.tcp.sack_offer = 1; + + inject_tcp_segment_with_opts(&s, TEST_PRIMARY_IF, + 0x0A0000A1U, 0x0A000001U, 40000, 8080, + 1, 0, TCP_FLAG_SYN, + opts, (uint8_t)sizeof(opts), NULL, 0); + + ck_assert_int_eq(ts->sock.tcp.ts_enabled, 0); +} +END_TEST + /* MSS option value 0 is ignored (falls back to default) */ START_TEST(test_tcp_parse_options_mss_zero_ignored) { diff --git a/src/wolfip.c b/src/wolfip.c index 5f595f0e..d64846d9 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -2878,7 +2878,7 @@ static void tcp_parse_options(const struct wolfIP_tcp_seg *tcp, uint32_t frame_l po->sack_count++; } } - } else if (kind == TCP_OPTION_TS && olen >= TCP_OPTION_TS_LEN) { + } else if (kind == TCP_OPTION_TS && olen == TCP_OPTION_TS_LEN) { uint32_t val, ecr; memcpy(&val, opt + 2, sizeof(val)); memcpy(&ecr, opt + 6, sizeof(ecr)); From 24d86d52645c7ae867f5786aa328ce64362b4388 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 18:56:51 +0200 Subject: [PATCH 23/24] tftp: reject Windows drive specifiers and NTFS ADS in filename safety check wolftftp_filename_is_safe() blocked leading '/' and '\' absolute paths and '..' components, but a Windows drive-letter path ("C:\windows\system32") or a drive-relative path ("C:foo") slipped through: the first segment is "C:", which is neither a separator-led absolute path nor a ".." component, so the name reached io.open(). NTFS alternate data streams ("name:stream") had the same gap. All three hinge on ':', which is never valid in a portable filename, so reject it wherever it appears. Adds the drive-letter, drive-relative, and ADS cases to test_tftp_parse_request_rejects_path_traversal; they fail before the fix (the names are accepted) and pass after, on both the RRQ and WRQ paths. --- src/test/unit/unit_tests_tftp.c | 3 +++ src/tftp/wolftftp.c | 19 ++++++++++++++----- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/test/unit/unit_tests_tftp.c b/src/test/unit/unit_tests_tftp.c index a4a6fb5c..fb860528 100644 --- a/src/test/unit/unit_tests_tftp.c +++ b/src/test/unit/unit_tests_tftp.c @@ -346,6 +346,9 @@ START_TEST(test_tftp_parse_request_rejects_path_traversal) "fw/../../secret", /* embedded traversal */ "images/..", /* trailing traversal component */ "a/..\\b", /* backslash-separated traversal*/ + "C:\\windows\\sys", /* windows drive-letter absolute*/ + "C:fw.bin", /* windows drive-relative path */ + "fw.bin:stream", /* ntfs alternate data stream */ }; static const char *good[] = { "fw.bin", /* ordinary name */ diff --git a/src/tftp/wolftftp.c b/src/tftp/wolftftp.c index 6ec3ef76..a43723c8 100644 --- a/src/tftp/wolftftp.c +++ b/src/tftp/wolftftp.c @@ -273,11 +273,12 @@ static int wolftftp_build_request(uint8_t *buf, uint16_t max_len, uint16_t opcod } /* Reject filenames that could escape the integrator's namespace: any absolute - * path (leading '/' or '\') and any ".." path component. The library hands the - * name straight to io.open(), and TFTP is unauthenticated, so a naive - * filesystem-backed open() would otherwise be exposed to traversal. The check - * is component-aware: a ".." segment between separators (or as the whole name) - * is rejected, but dots inside a name ("fw..bin") are fine. */ + * path (leading '/' or '\', or a Windows drive specifier like "C:\...") and any + * ".." path component. The library hands the name straight to io.open(), and + * TFTP is unauthenticated, so a naive filesystem-backed open() would otherwise + * be exposed to traversal. The check is component-aware: a ".." segment between + * separators (or as the whole name) is rejected, but dots inside a name + * ("fw..bin") are fine. */ static int wolftftp_filename_is_safe(const char *name) { const char *seg = name; @@ -287,6 +288,14 @@ static int wolftftp_filename_is_safe(const char *name) return 0; if (name[0] == '/' || name[0] == '\\') return 0; + /* ':' is never valid in a portable filename; on Windows it introduces a + * drive specifier ("C:\windows", "C:foo") or an NTFS alternate data stream + * ("name:stream"), either of which would sidestep the leading-separator + * check above. Reject it wherever it appears. */ + for (c = name; *c != '\0'; c++) { + if (*c == ':') + return 0; + } for (c = name; ; c++) { if (*c == '/' || *c == '\\' || *c == '\0') { size_t seglen = (size_t)(c - seg); From f2644e584bf0f019b6bc4ac151171c82dcae2f1a Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 4 Jun 2026 18:56:51 +0200 Subject: [PATCH 24/24] ci: tolerate gcov -block lines in autocov so gcovr 7.0 does not abort The autocov job installs gcovr 7.0 from apt (ubuntu-noble). That version does not recognize the ":-block " lines emitted by the runner's gcov and aborts with "UnknownLineType ... -block 0" before any coverage gate runs, failing the whole job. gcovr 7.1+ parses these lines, which is why it reproduces only on the CI toolchain. Add --gcov-ignore-parse-errors=all to every gcovr invocation (the Makefile coverage/autocov targets and the workflow JSON step), alongside the existing --gcov-ignore-errors flag. The auxiliary -block lines are skipped while the canonical "count:lineno:" and function records still parse, so function coverage is unchanged: default 224/224, vlan 227/227, multicast 238/238. --- .github/workflows/wolfip-autocov.yml | 2 +- Makefile | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/wolfip-autocov.yml b/.github/workflows/wolfip-autocov.yml index 6cd0ccf5..b681c7ec 100644 --- a/.github/workflows/wolfip-autocov.yml +++ b/.github/workflows/wolfip-autocov.yml @@ -26,7 +26,7 @@ jobs: - name: Generate coverage JSON run: | - gcovr -r . --exclude "src/test/unit/unit.c" --json -o build/coverage/coverage.json + gcovr -r . --exclude "src/test/unit/unit.c" --gcov-ignore-parse-errors=all --json -o build/coverage/coverage.json - name: Enforce 100% function coverage for src/wolfip.c run: | diff --git a/Makefile b/Makefile index bf5096ba..d3e6dcd1 100644 --- a/Makefile +++ b/Makefile @@ -562,6 +562,7 @@ cov: unit $(COV_UNIT) @mkdir -p build/coverage @gcovr -r . --exclude "src/test/unit/.*" \ --gcov-ignore-errors=no_working_dir_found \ + --gcov-ignore-parse-errors=all \ --merge-mode-functions=merge-use-line-min \ --html-details -o build/coverage/index.html @$(OPEN_CMD) build/coverage/index.html @@ -576,6 +577,7 @@ autocov: unit $(COV_UNIT) @mkdir -p build/coverage @gcovr -r . --exclude "src/test/unit/.*" \ --gcov-ignore-errors=no_working_dir_found \ + --gcov-ignore-parse-errors=all \ --merge-mode-functions=merge-use-line-min \ --html-details -o build/coverage/index.html @@ -587,6 +589,7 @@ autocov-multicast: unit-multicast $(COV_MCAST_UNIT) @mkdir -p build/coverage @gcovr -r . --exclude "src/test/unit/.*" \ --gcov-ignore-errors=no_working_dir_found \ + --gcov-ignore-parse-errors=all \ --merge-mode-functions=merge-use-line-min \ --html-details -o build/coverage/multicast.html @@ -598,6 +601,7 @@ cov-multicast: unit-multicast $(COV_MCAST_UNIT) @mkdir -p build/coverage @gcovr -r . --exclude "src/test/unit/.*" \ --gcov-ignore-errors=no_working_dir_found \ + --gcov-ignore-parse-errors=all \ --merge-mode-functions=merge-use-line-min \ --html-details -o build/coverage/multicast.html @$(OPEN_CMD) build/coverage/multicast.html @@ -620,6 +624,7 @@ cov-vlan: unit-vlan $(COV_VLAN_UNIT) @mkdir -p build/coverage @gcovr -r . --exclude "src/test/unit/.*" \ --gcov-ignore-errors=no_working_dir_found \ + --gcov-ignore-parse-errors=all \ --merge-mode-functions=merge-use-line-min \ --html-details -o build/coverage/vlan.html @$(OPEN_CMD) build/coverage/vlan.html @@ -632,6 +637,7 @@ autocov-vlan: unit-vlan $(COV_VLAN_UNIT) @mkdir -p build/coverage @gcovr -r . --exclude "src/test/unit/.*" \ --gcov-ignore-errors=no_working_dir_found \ + --gcov-ignore-parse-errors=all \ --merge-mode-functions=merge-use-line-min \ --html-details -o build/coverage/vlan.html