From 407178b5a1d7488caab1e16cbcbbef49f9b3448e Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 12:58:40 +0200 Subject: [PATCH 01/14] F-6209: keep raw/packet TX descriptor queued on driver backpressure flush_raw_tx and flush_packet_tx discarded the return of wolfIP_ll_send_frame and popped the descriptor unconditionally, so a retryable -WOLFIP_EAGAIN (loopback queue full, driver TX ring full) silently dropped a frame that wolfIP_sock_sendto had already reported as queued. Mirror flush_datagram_tx: break out of the drain loop on a negative send result, leaving the descriptor at the FIFO head for the next poll cycle. Adds regression tests driving the mock link into -WOLFIP_EAGAIN for both a raw socket and an AF_PACKET socket: the frame must not be transmitted on the backpressured poll, the descriptor must survive in the TX FIFO, and the next poll must retransmit it intact. --- src/test/unit/unit.c | 2 + src/test/unit/unit_tests_proto.c | 116 +++++++++++++++++++++++++++++++ src/wolfip.c | 12 +++- 3 files changed, 128 insertions(+), 2 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 3fc8ce70..f416708c 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -851,6 +851,7 @@ Suite *wolf_suite(void) 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); + tcase_add_test(tc_proto, test_regression_raw_socket_tx_eagain_keeps_descriptor); tcase_add_test(tc_proto, test_raw_socket_sendto_short_addrlen_returns_einval); tcase_add_test(tc_proto, test_raw_socket_sendto_wrong_family_returns_einval); tcase_add_test(tc_proto, test_raw_socket_sendto_payload_too_large_for_ip_header_returns_einval); @@ -860,6 +861,7 @@ 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); + tcase_add_test(tc_proto, test_regression_packet_socket_tx_eagain_keeps_descriptor); #if WOLFIP_PACKET_SOCKETS tcase_add_test(tc_proto, test_packet_socket_tx_filter_block_does_not_resend); #endif diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index ee4ed934..18fa8564 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -7611,6 +7611,60 @@ START_TEST(test_regression_raw_socket_send_ip_id_network_byte_order) } END_TEST +/* F-6209: a driver -WOLFIP_EAGAIN from the link-layer send must leave the + * descriptor queued for retry on the next poll, not silently drop the frame + * (flush_raw_tx used to pop unconditionally, unlike flush_datagram_tx). */ +START_TEST(test_regression_raw_socket_tx_eagain_keeps_descriptor) +{ + struct wolfIP s; + int sd; + uint8_t payload[6] = {0x11, 0x22, 0x33, 0x44, 0x55, 0x66}; + struct wolfIP_sockaddr_in sin; + uint32_t dst_ip = 0x0A00000CU; + uint8_t nh_mac[6] = {0x2A, 0x2B, 0x2C, 0x2D, 0x2E, 0x2F}; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + s.arp.neighbors[0].ip = dst_ip; + s.arp.neighbors[0].if_idx = TEST_PRIMARY_IF; + memcpy(s.arp.neighbors[0].mac, nh_mac, sizeof(nh_mac)); + + sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_RAW, WI_IPPROTO_UDP); + ck_assert_int_ge(sd, 0); + + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_addr.s_addr = ee32(dst_ip); + + ck_assert_int_eq(wolfIP_sock_sendto(&s, sd, payload, sizeof(payload), 0, + (struct wolfIP_sockaddr *)&sin, sizeof(sin)), + (int)sizeof(payload)); + + mock_link_capture_reset(); + mock_send_eagain_armed = 1; + + /* The driver reports backpressure: nothing goes on the wire, and the + * descriptor must stay queued at the FIFO head for the next poll. */ + wolfIP_poll(&s, 0); + ck_assert_uint_eq(mock_sent_frames_count, 0U); + ck_assert_ptr_nonnull(fifo_peek(&s.rawsockets[SOCKET_UNMARK(sd)].txbuf)); + + /* Next poll: the queued frame is retransmitted intact. */ + mock_link_capture_reset(); + wolfIP_poll(&s, 0); + ck_assert_uint_eq(mock_sent_frames_count, 1U); + ck_assert_uint_eq(last_frame_sent_size, + ETH_HEADER_LEN + IP_HEADER_LEN + sizeof(payload)); + { + struct wolfIP_ip_packet *sent = (struct wolfIP_ip_packet *)last_frame_sent; + ck_assert_mem_eq(sent->data, payload, sizeof(payload)); + ck_assert_mem_eq(sent->eth.dst, nh_mac, 6); + } +} +END_TEST + START_TEST(test_raw_socket_sendto_short_addrlen_returns_einval) { struct wolfIP s; @@ -7865,6 +7919,68 @@ START_TEST(test_packet_socket_send_frame) } END_TEST +/* F-6209: same retry contract for packet sockets: a driver -WOLFIP_EAGAIN + * must keep the frame queued, not drop it. */ +START_TEST(test_regression_packet_socket_tx_eagain_keeps_descriptor) +{ + struct wolfIP s; + int sd; + struct wolfIP_sockaddr_ll sll; + struct wolfIP_sockaddr_ll bind_sll; + uint8_t frame_buf[ETH_HEADER_LEN + 8]; + struct wolfIP_eth_frame *ethf = (struct wolfIP_eth_frame *)frame_buf; + + 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); + + memset(frame_buf, 0, sizeof(frame_buf)); + memcpy(ethf->dst, "\xff\xff\xff\xff\xff\xff", 6); + memcpy(ethf->src, "\x00\x00\x00\x00\x00\x01", 6); + ethf->type = ee16(ETH_TYPE_IP); + memset(ethf->data, 0xCD, 8); + + ck_assert_int_eq(wolfIP_sock_sendto(&s, sd, frame_buf, sizeof(frame_buf), 0, + (struct wolfIP_sockaddr *)&sll, sizeof(sll)), + (int)sizeof(frame_buf)); + + mock_link_capture_reset(); + mock_send_eagain_armed = 1; + + wolfIP_poll(&s, 0); + ck_assert_uint_eq(mock_sent_frames_count, 0U); + ck_assert_ptr_nonnull( + fifo_peek(&s.packetsockets[SOCKET_UNMARK(sd)].txbuf)); + + mock_link_capture_reset(); + wolfIP_poll(&s, 0); + ck_assert_uint_eq(mock_sent_frames_count, 1U); + ck_assert_uint_eq(last_frame_sent_size, sizeof(frame_buf)); + { + struct wolfIP_eth_frame *sent = (struct wolfIP_eth_frame *)last_frame_sent; + ck_assert_mem_eq(sent->data, ethf->data, 8); + } +} +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 diff --git a/src/wolfip.c b/src/wolfip.c index 3951be7f..7bb203ff 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -11619,7 +11619,11 @@ static void flush_raw_tx(struct wolfIP *s) break; eth_output_add_header(s, tx_if, r->nexthop_mac, &ip->eth, ETH_TYPE_IP); #endif - wolfIP_ll_send_frame(s, tx_if, ip, desc->len); + /* Mirror flush_datagram_tx: on driver backpressure/hard error + * keep the descriptor at the FIFO head so the next poll retries + * it instead of silently dropping the frame. */ + if (wolfIP_ll_send_frame(s, tx_if, ip, desc->len) < 0) + break; fifo_pop(&r->txbuf); desc = fifo_peek(&r->txbuf); (void)nexthop; @@ -11655,7 +11659,11 @@ static void flush_packet_tx(struct wolfIP *s) desc = fifo_peek(&p->txbuf); continue; } - wolfIP_ll_send_frame(s, tx_if, frame, desc->len); + /* Mirror flush_datagram_tx: on driver backpressure/hard error + * keep the descriptor at the FIFO head so the next poll retries + * it instead of silently dropping the frame. */ + if (wolfIP_ll_send_frame(s, tx_if, frame, desc->len) < 0) + break; fifo_pop(&p->txbuf); desc = fifo_peek(&p->txbuf); } From 5815e8ac754a6dcb89b88aa0be665336d7042c1f Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 13:01:55 +0200 Subject: [PATCH 02/14] F-10259: clear raw/packet socket events before dispatching the callback The raw and AF_PACKET loops in handle_socket_callbacks() cleared r->events / p->events after the callback returned, inverting the order dispatch_events() (UDP/ICMP) and the TCP path use. An event raised on the same slot while the callback is executing (e.g. the callback closes the socket and re-opens a new one in the reused slot) was wiped by the stale post-callback clear, so a consumer waiting on it never woke. Snapshot the events and clear the field before invoking the callback, mirroring dispatch_events(). Adds regression tests for both socket types: a callback that closes and re-opens the socket in place raises CB_EVENT_WRITABLE on the reused slot; it must survive the poll and wake the reopened socket's callback on the next one. --- src/test/unit/unit.c | 2 + src/test/unit/unit_tests_poll_dispatcher.c | 115 +++++++++++++++++++++ src/wolfip.c | 14 ++- 3 files changed, 129 insertions(+), 2 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index f416708c..65d5016d 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -1362,9 +1362,11 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_poll_udp_socket_callback_dispatched); #if WOLFIP_RAWSOCKETS tcase_add_test(tc_core, test_poll_raw_socket_callback_dispatched); + tcase_add_test(tc_core, test_poll_raw_socket_callback_reraised_event_survives); #endif /* WOLFIP_RAWSOCKETS */ #if WOLFIP_PACKET_SOCKETS tcase_add_test(tc_core, test_poll_packet_socket_callback_dispatched); + tcase_add_test(tc_core, test_poll_packet_socket_callback_reraised_event_survives); #endif /* WOLFIP_PACKET_SOCKETS */ tcase_add_test(tc_core, test_poll_tx_tcp_pkt_flag_sent_desc_skipped); tcase_add_test(tc_core, test_poll_tx_tcp_arp_miss_emits_arp_request); diff --git a/src/test/unit/unit_tests_poll_dispatcher.c b/src/test/unit/unit_tests_poll_dispatcher.c index 00b9be4f..7583b5be 100644 --- a/src/test/unit/unit_tests_poll_dispatcher.c +++ b/src/test/unit/unit_tests_poll_dispatcher.c @@ -479,6 +479,121 @@ START_TEST(test_poll_packet_socket_callback_dispatched) END_TEST #endif /* WOLFIP_PACKET_SOCKETS */ +/* F-10259: an event raised on a raw/packet socket slot while the callback is + * still on the stack (callback closes the socket and re-opens a new one in + * the same slot) must survive dispatch. The old loops cleared the events + * field after the callback returned, wiping the new socket's event so a + * consumer waiting on it never woke. */ +static struct wolfIP *f10259_stack; +static int f10259_reentered; + +#if WOLFIP_RAWSOCKETS +static int f10259_reopen_fd; +static void f10259_raw_reopen_cb(int sock_fd, uint16_t events, void *arg) +{ + (void)events; + (void)arg; + if (!f10259_reentered) { + f10259_reentered = 1; + wolfIP_sock_close(f10259_stack, sock_fd); + f10259_reopen_fd = wolfIP_sock_socket(f10259_stack, AF_INET, + IPSTACK_SOCK_RAW, WI_IPPROTO_ICMP); + if (f10259_reopen_fd >= 0) { + wolfIP_register_callback(f10259_stack, f10259_reopen_fd, + test_socket_cb, NULL); + /* Event raised on the reused slot while the old callback is + * still executing. */ + f10259_stack->rawsockets[SOCKET_UNMARK(f10259_reopen_fd)].events |= + CB_EVENT_WRITABLE; + } + } +} +START_TEST(test_poll_raw_socket_callback_reraised_event_survives) +{ + struct wolfIP s; + int raw_sd; + + wolfIP_init(&s); + mock_link_init(&s); + socket_cb_calls = 0; + socket_cb_last_fd = -1; + f10259_stack = &s; + f10259_reentered = 0; + f10259_reopen_fd = -1; + + raw_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_RAW, WI_IPPROTO_ICMP); + ck_assert_int_ge(raw_sd, 0); + wolfIP_register_callback(&s, raw_sd, f10259_raw_reopen_cb, NULL); + s.rawsockets[SOCKET_UNMARK(raw_sd)].events = CB_EVENT_READABLE; + + (void)wolfIP_poll(&s, 100); + /* The reopened socket reuses slot 0; the event raised during dispatch + * must not be wiped by the old iteration's clear. */ + ck_assert_int_ge(f10259_reopen_fd, 0); + ck_assert_int_eq(f10259_reopen_fd, raw_sd); + ck_assert(s.rawsockets[SOCKET_UNMARK(f10259_reopen_fd)].events & + CB_EVENT_WRITABLE); + + /* Next poll: the reopened socket's own callback is woken by it. */ + socket_cb_calls = 0; + (void)wolfIP_poll(&s, 200); + ck_assert_int_eq(socket_cb_calls, 1); + ck_assert_int_eq(socket_cb_last_fd, f10259_reopen_fd); +} +END_TEST +#endif /* WOLFIP_RAWSOCKETS */ + +#if WOLFIP_PACKET_SOCKETS +static int f10259_pkt_reopen_fd; +static void f10259_pkt_reopen_cb(int sock_fd, uint16_t events, void *arg) +{ + (void)events; + (void)arg; + if (!f10259_reentered) { + f10259_reentered = 1; + wolfIP_sock_close(f10259_stack, sock_fd); + f10259_pkt_reopen_fd = wolfIP_sock_socket(f10259_stack, AF_PACKET, + IPSTACK_SOCK_RAW, ee16(ETH_TYPE_IP)); + if (f10259_pkt_reopen_fd >= 0) { + wolfIP_register_callback(f10259_stack, f10259_pkt_reopen_fd, + test_socket_cb, NULL); + f10259_stack->packetsockets[SOCKET_UNMARK(f10259_pkt_reopen_fd)].events |= + CB_EVENT_WRITABLE; + } + } +} +START_TEST(test_poll_packet_socket_callback_reraised_event_survives) +{ + struct wolfIP s; + int pkt_sd; + + wolfIP_init(&s); + mock_link_init(&s); + socket_cb_calls = 0; + socket_cb_last_fd = -1; + f10259_stack = &s; + f10259_reentered = 0; + f10259_pkt_reopen_fd = -1; + + pkt_sd = wolfIP_sock_socket(&s, AF_PACKET, IPSTACK_SOCK_RAW, ee16(ETH_TYPE_IP)); + ck_assert_int_ge(pkt_sd, 0); + wolfIP_register_callback(&s, pkt_sd, f10259_pkt_reopen_cb, NULL); + s.packetsockets[SOCKET_UNMARK(pkt_sd)].events = CB_EVENT_READABLE; + + (void)wolfIP_poll(&s, 100); + ck_assert_int_ge(f10259_pkt_reopen_fd, 0); + ck_assert_int_eq(f10259_pkt_reopen_fd, pkt_sd); + ck_assert(s.packetsockets[SOCKET_UNMARK(f10259_pkt_reopen_fd)].events & + CB_EVENT_WRITABLE); + + socket_cb_calls = 0; + (void)wolfIP_poll(&s, 200); + ck_assert_int_eq(socket_cb_calls, 1); + ck_assert_int_eq(socket_cb_last_fd, f10259_pkt_reopen_fd); +} +END_TEST +#endif /* WOLFIP_PACKET_SOCKETS */ + /* ------------------------------------------------------------------ */ /* TCP TX loop */ /* ------------------------------------------------------------------ */ diff --git a/src/wolfip.c b/src/wolfip.c index 7bb203ff..c27571e0 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -11269,8 +11269,13 @@ static void handle_socket_callbacks(struct wolfIP *s) for (i = 0; i < WOLFIP_MAX_RAWSOCKETS; i++) { struct rawsocket *r = &s->rawsockets[i]; if (r->used && (r->callback) && (r->events)) { - r->callback(i | MARK_RAW_SOCKET, r->events, r->callback_arg); + /* Snapshot and clear before the callback (as dispatch_events + * does): the callback may re-enter the stack and raise events + * on this same slot (e.g. close + re-socket in place); a + * post-callback clear would wipe them. */ + uint16_t events = r->events; r->events = 0; + r->callback(i | MARK_RAW_SOCKET, events, r->callback_arg); } } #endif @@ -11278,8 +11283,13 @@ static void handle_socket_callbacks(struct wolfIP *s) for (i = 0; i < WOLFIP_MAX_PACKETSOCKETS; i++) { struct packetsocket *p = &s->packetsockets[i]; if (p->used && (p->callback) && (p->events)) { - p->callback(i | MARK_PACKET_SOCKET, p->events, p->callback_arg); + /* Snapshot and clear before the callback (as dispatch_events + * does): the callback may re-enter the stack and raise events + * on this same slot (e.g. close + re-socket in place); a + * post-callback clear would wipe them. */ + uint16_t events = p->events; p->events = 0; + p->callback(i | MARK_PACKET_SOCKET, events, p->callback_arg); } } #endif From 855ad2c6b99cb8262d0d40b9e9178d1312a78a83 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 13:09:33 +0200 Subject: [PATCH 03/14] F-10260: bound the PTR name walk to the RDATA, not the whole message dns_callback validated that the complete RDATA lies inside the DNS message but then passed the full message length to dns_copy_name, whose label bound checks use that length. The inline portion of a PTR answer name could therefore continue past the RDATA into the following record: with rdlength 1 holding a label-length byte of 3 and "foo" in the bytes after, the parser returned "foo" and invoked dns_ptr_cb with a name the RDATA never contained. dns_copy_name now takes the RDATA edge (rdata_end) as a sixth argument: the initial inline portion (labels, terminators and both bytes of a compression pointer) must fit in the RDATA, while pointer targets and the post-jump name portion remain validated against the message length, as RFC 1035 s4.1.4 allows. The PTR arm passes pos + rdlen; direct buffer unit tests pass rdata_end == len (behavior unchanged). Adds the finding's trigger as a regression test (undersized RDATA followed by name-looking bytes must leave the query pending) plus a companion test that a PTR RDATA legitimately ending in a compression pointer still parses. --- src/test/unit/unit.c | 2 + src/test/unit/unit_tests_api.c | 6 +- src/test/unit/unit_tests_dns_edges.c | 145 +++++++++++++++++++++++++-- src/wolfip.c | 33 +++++- 4 files changed, 170 insertions(+), 16 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 65d5016d..5c4c3c2b 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -1550,6 +1550,8 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_dns_skip_name_label_past_end); tcase_add_test(tc_core, test_dns_copy_name_second_label_separator_and_label_fit); tcase_add_test(tc_core, test_dns_callback_ptr_bad_copy_name_stays_pending); + tcase_add_test(tc_core, test_dns_callback_ptr_name_beyond_rdata_rejected); + tcase_add_test(tc_core, test_dns_callback_ptr_rdata_ends_in_pointer_ok); tcase_add_test(tc_core, test_dns_copy_name_jumped_no_pos_increment); tcase_add_test(tc_core, test_dns_send_query_socket_alloc_failure); /* --- unit_tests_misc_edges.c (75 tests) --- */ diff --git a/src/test/unit/unit_tests_api.c b/src/test/unit/unit_tests_api.c index 621643b9..627cbbba 100644 --- a/src/test/unit/unit_tests_api.c +++ b/src/test/unit/unit_tests_api.c @@ -1098,14 +1098,14 @@ START_TEST(test_dns_skip_and_copy_name) ret = dns_skip_name(buf, sizeof(buf), 0); ck_assert_int_eq(ret, pos); - ret = dns_copy_name(buf, sizeof(buf), 0, out, sizeof(out)); + ret = dns_copy_name(buf, sizeof(buf), 0, out, sizeof(out), sizeof(buf)); ck_assert_int_eq(ret, 0); ck_assert_str_eq(out, "www.example.com"); /* add a pointer to the name at offset 0 */ buf[pos++] = 0xC0; buf[pos++] = 0x00; - ret = dns_copy_name(buf, sizeof(buf), pos - 2, out, sizeof(out)); + ret = dns_copy_name(buf, sizeof(buf), pos - 2, out, sizeof(out), sizeof(buf)); ck_assert_int_eq(ret, 0); ck_assert_str_eq(out, "www.example.com"); @@ -1115,7 +1115,7 @@ START_TEST(test_dns_skip_and_copy_name) buf[pos++] = (uint8_t)(ptr_pos + 2); buf[pos++] = 3; memcpy(&buf[pos], "bad", 3); pos += 3; buf[pos++] = 0; - ret = dns_copy_name(buf, pos, ptr_pos, out, sizeof(out)); + ret = dns_copy_name(buf, pos, ptr_pos, out, sizeof(out), pos); ck_assert_int_eq(ret, -1); } END_TEST diff --git a/src/test/unit/unit_tests_dns_edges.c b/src/test/unit/unit_tests_dns_edges.c index f467d875..97619ef6 100644 --- a/src/test/unit/unit_tests_dns_edges.c +++ b/src/test/unit/unit_tests_dns_edges.c @@ -414,7 +414,7 @@ START_TEST(test_dns_copy_name_label_too_big_for_output) int ret; /* out_len == 2: 0 + 2 >= 2 → label-bound guard fires → -1 */ - ret = dns_copy_name(buf, (int)sizeof(buf), 0, out, sizeof(out)); + ret = dns_copy_name(buf, (int)sizeof(buf), 0, out, sizeof(out), (int)sizeof(buf)); ck_assert_int_eq(ret, -1); } END_TEST @@ -431,7 +431,7 @@ START_TEST(test_dns_copy_name_zero_out_len_rejects_terminator_write) char out[1]; /* not written; placeholder */ int ret; - ret = dns_copy_name(buf, (int)sizeof(buf), 0, out, 0); + ret = dns_copy_name(buf, (int)sizeof(buf), 0, out, 0, (int)sizeof(buf)); ck_assert_int_eq(ret, -1); } END_TEST @@ -448,7 +448,7 @@ START_TEST(test_dns_copy_name_ptr_at_end_of_buffer) char out[32]; int ret; - ret = dns_copy_name(buf, 3, 2, out, sizeof(out)); + ret = dns_copy_name(buf, 3, 2, out, sizeof(out), 3); ck_assert_int_eq(ret, -1); } END_TEST @@ -463,7 +463,7 @@ START_TEST(test_dns_copy_name_label_past_end) char out[32]; int ret; - ret = dns_copy_name(buf, (int)sizeof(buf), 0, out, sizeof(out)); + ret = dns_copy_name(buf, (int)sizeof(buf), 0, out, sizeof(out), (int)sizeof(buf)); ck_assert_int_eq(ret, -1); } END_TEST @@ -484,7 +484,7 @@ START_TEST(test_dns_copy_name_separator_overflow) char out[3]; int ret; - ret = dns_copy_name(buf, (int)sizeof(buf), 0, out, sizeof(out)); + ret = dns_copy_name(buf, (int)sizeof(buf), 0, out, sizeof(out), (int)sizeof(buf)); ck_assert_int_eq(ret, -1); } END_TEST @@ -501,7 +501,7 @@ START_TEST(test_dns_copy_name_label_overflow_output) char out[3]; int ret; - ret = dns_copy_name(buf, (int)sizeof(buf), 0, out, sizeof(out)); + ret = dns_copy_name(buf, (int)sizeof(buf), 0, out, sizeof(out), (int)sizeof(buf)); ck_assert_int_eq(ret, -1); } END_TEST @@ -536,13 +536,13 @@ START_TEST(test_dns_copy_name_second_label_separator_and_label_fit) int ret; /* Should succeed with enough room */ - ret = dns_copy_name(buf, (int)sizeof(buf), 0, out_ok, sizeof(out_ok)); + ret = dns_copy_name(buf, (int)sizeof(buf), 0, out_ok, sizeof(out_ok), (int)sizeof(buf)); ck_assert_int_eq(ret, 0); ck_assert_str_eq(out_ok, "ab.cd"); /* Should fail: out_len == 5, after "ab" o=2, need o+1 < 5 (ok), * then o+c = 2+1+2 = 5 >= 5 → overflow at label copy */ - ret = dns_copy_name(buf, (int)sizeof(buf), 0, out_small, sizeof(out_small)); + ret = dns_copy_name(buf, (int)sizeof(buf), 0, out_small, sizeof(out_small), (int)sizeof(buf)); ck_assert_int_eq(ret, -1); } END_TEST @@ -611,6 +611,133 @@ START_TEST(test_dns_callback_ptr_bad_copy_name_stays_pending) } END_TEST +/* ------------------------------------------------------------------ * + * F-10260: a PTR RDATA whose name encoding does not fit in the declared + * rdlength must be rejected. The old code bounded dns_copy_name by the + * full message length, so an inline label could continue past the RDATA + * into the following record and dns_ptr_cb was invoked with a name the + * RDATA never contained. The bytes after the 1-byte RDATA spell "foo." + * but the RDATA itself holds only the label-length byte 3. + * ------------------------------------------------------------------ */ +START_TEST(test_dns_callback_ptr_name_beyond_rdata_rejected) +{ + struct wolfIP s; + uint8_t response[128]; + struct dns_header *hdr = (struct dns_header *)response; + struct dns_question *q; + struct dns_rr *rr; + int pos; + + wolfIP_init(&s); + mock_link_init(&s); + s.dns_server = 0x0A000001U; + arm_dns_query(&s, 0xBBBB, dns_qname_a, (int)sizeof(dns_qname_a), DNS_PTR); + s.dns_ptr_cb = test_dns_ptr_cb; + s.dns_lookup_cb = NULL; + s.dns_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); + ck_assert_int_gt(s.dns_udp_sd, 0); + + memset(response, 0, sizeof(response)); + hdr->id = ee16(s.dns_id); + hdr->flags = ee16(0x8100); + hdr->qdcount = ee16(1); + hdr->ancount = ee16(1); + pos = (int)sizeof(struct dns_header); + response[pos++] = 1; response[pos++] = 'a'; response[pos++] = 0; + q = (struct dns_question *)(response + pos); + q->qtype = ee16(DNS_PTR); + q->qclass = ee16(DNS_CLASS_IN); + pos += (int)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_PTR); + rr->class = ee16(DNS_CLASS_IN); + rr->ttl = ee32(60); + rr->rdlength = ee16(1); + pos += (int)sizeof(struct dns_rr); + + /* RDATA is a single byte claiming a 3-char label; "foo" + terminator + * sit in the bytes that follow the RDATA (outside it). */ + response[pos++] = 3; + response[pos++] = 'f'; + response[pos++] = 'o'; + response[pos++] = 'o'; + response[pos++] = 0; + + 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); + + /* The name does not fit the RDATA → copy fails → the bogus name must + * not be delivered and the query stays pending. */ + ck_assert_uint_eq(s.dns_id, 0xBBBB); + ck_assert_int_eq(s.dns_query_type, DNS_QUERY_TYPE_PTR); +} +END_TEST + +/* ------------------------------------------------------------------ * + * F-10260 companion: a PTR RDATA that legitimately ends in a compression + * pointer (RFC 1035 s4.1.4 allows the pointer to reference any offset in + * the message) must still parse. Guards the rdata_end bound from being + * applied past the pointer jump. + * ------------------------------------------------------------------ */ +START_TEST(test_dns_callback_ptr_rdata_ends_in_pointer_ok) +{ + struct wolfIP s; + uint8_t response[128]; + struct dns_header *hdr = (struct dns_header *)response; + struct dns_question *q; + struct dns_rr *rr; + int pos; + + wolfIP_init(&s); + mock_link_init(&s); + s.dns_server = 0x0A000001U; + arm_dns_query(&s, 0xBBBB, dns_qname_a, (int)sizeof(dns_qname_a), DNS_PTR); + s.dns_ptr_cb = test_dns_ptr_cb; + s.dns_lookup_cb = NULL; + s.dns_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); + ck_assert_int_gt(s.dns_udp_sd, 0); + + memset(response, 0, sizeof(response)); + hdr->id = ee16(s.dns_id); + hdr->flags = ee16(0x8100); + hdr->qdcount = ee16(1); + hdr->ancount = ee16(1); + pos = (int)sizeof(struct dns_header); + response[pos++] = 1; response[pos++] = 'a'; response[pos++] = 0; + q = (struct dns_question *)(response + pos); + q->qtype = ee16(DNS_PTR); + q->qclass = ee16(DNS_CLASS_IN); + pos += (int)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_PTR); + rr->class = ee16(DNS_CLASS_IN); + rr->ttl = ee32(60); + /* RDATA: label "x" then a pointer to the question name ("a") → "x.a" */ + rr->rdlength = ee16(4); + pos += (int)sizeof(struct dns_rr); + response[pos++] = 1; + response[pos++] = 'x'; + response[pos++] = 0xC0; + response[pos++] = (uint8_t)sizeof(struct dns_header); + + 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); + + /* Valid name → ptr_cb called → query aborted */ + ck_assert_uint_eq(s.dns_id, 0U); +} +END_TEST + /* ------------------------------------------------------------------ * * dns_copy_name: jumped == 1, so pos is NOT incremented after reading * the NUL terminator (line 8813-8814 true branch). @@ -630,7 +757,7 @@ START_TEST(test_dns_copy_name_jumped_no_pos_increment) /* Start at the compression pointer (offset 1). * The pointer lands at offset 0 which is '\0', so jumped == 1 and * the NUL-terminator branch sets out[0]='\0' without touching pos. */ - ret = dns_copy_name(buf, (int)sizeof(buf), 1, out, sizeof(out)); + ret = dns_copy_name(buf, (int)sizeof(buf), 1, out, sizeof(out), (int)sizeof(buf)); ck_assert_int_eq(ret, 0); ck_assert_uint_eq((uint8_t)out[0], 0); } diff --git a/src/wolfip.c b/src/wolfip.c index c27571e0..de808b87 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -10774,15 +10774,31 @@ static int dns_question_matches(struct wolfIP *s, const uint8_t *buf, int len, sizeof(struct dns_question)) == 0; } +/* len bounds the whole message (compression-pointer targets and the + * post-jump name portion are validated against it); rdata_end bounds the + * initial inline portion of the name, which per RFC 1035 must be encoded + * within the record's RDATA. Pass rdata_end == len when the name encoding + * is not part of an RDATA (unit tests on bare buffers). */ static int dns_copy_name(const uint8_t *buf, int len, int offset, char *out, - size_t out_len) + size_t out_len, int rdata_end) { int pos = offset; size_t o = 0; int loop = 0; int jumped = 0; - while (pos < len && loop++ < len) { - uint8_t c = buf[pos]; + while (loop++ < len) { + int bound; + uint8_t c; + /* The inline portion stops at the RDATA edge; once a compression + * pointer has jumped elsewhere in the message the name is bounded + * by the message length. */ + if (jumped) + bound = len; + else + bound = (rdata_end < len) ? rdata_end : len; + if (pos >= bound) + break; + c = buf[pos]; if (c == DNS_NAME_TERMINATOR) { if (!jumped) pos++; @@ -10798,6 +10814,10 @@ static int dns_copy_name(const uint8_t *buf, int len, int offset, char *out, int ptr_pos = pos; if (pos + 1 >= len) return -1; + /* The pointer is part of the inline encoding: both bytes must + * lie within the RDATA. */ + if (!jumped && pos + 2 > rdata_end) + return -1; { uint16_t ptr = ((c & DNS_COMPRESSION_OFFSET_MASK) << 8) | buf[pos + 1]; @@ -10811,6 +10831,10 @@ static int dns_copy_name(const uint8_t *buf, int len, int offset, char *out, pos++; if (pos + c > len) return -1; + /* An inline label (length byte + label bytes) must fit in the + * RDATA; do not let it continue into the following record. */ + if (!jumped && pos + c > rdata_end) + return -1; if (o != 0) { if (o + 1 >= out_len) return -1; @@ -11005,7 +11029,8 @@ void dns_callback(int dns_sd, uint16_t ev, void *arg) ee16(rr->type) == DNS_PTR && ee16(rr->class) == DNS_CLASS_IN) { if (dns_copy_name((const uint8_t *)buf, dns_len, pos, - s->dns_ptr_name, sizeof(s->dns_ptr_name)) == 0) { + s->dns_ptr_name, sizeof(s->dns_ptr_name), + pos + (int)rdlen) == 0) { if (s->dns_ptr_cb) s->dns_ptr_cb(s->dns_ptr_name); dns_abort_query(s); From f21c6da880bf61e298553c89c50184d824474050 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 13:10:16 +0200 Subject: [PATCH 04/14] F-9366: fix http_request.query comment to cover non-GET requests parse_http_request() splits the request target on '?' and populates req.query unconditionally, before the method is validated against GET/POST. A POST target with a query string ("POST /api?id=5") gets req.query filled exactly like a GET, so the "(for GET requests)" comment was false and could mislead a handler into skipping httpd_get_request_arg() on POST. --- src/http/httpd.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/http/httpd.h b/src/http/httpd.h index d4784196..cbadc1aa 100644 --- a/src/http/httpd.h +++ b/src/http/httpd.h @@ -41,7 +41,7 @@ struct httpd; struct http_request { char method[HTTP_METHOD_LEN]; // "GET", "POST", etc. char path[HTTP_PATH_LEN]; // URL path - char query[HTTP_QUERY_LEN]; // URL query string (for GET requests) + char query[HTTP_QUERY_LEN]; // URL query string, if present in the target char headers[HTTP_HEADERS_LEN]; // HTTP headers char body[HTTP_BODY_LEN]; // HTTP body (for POST requests) size_t body_len; From 8ac77177c74642f025e1ff520bb0f0bdebf5698b Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 13:19:48 +0200 Subject: [PATCH 05/14] F-6475: prove the IGMP anti-disclosure guards are actually tested test_multicast_igmp_query_spoofed_dropped only asserted that no frame was sent synchronously after each spoofed query - but IGMP reports are always deferred to a timer (RFC 3376 s5.2), so the assertion was trivially true whether the query was dropped or accepted. Deleting the TTL guard (ip->ttl != 1) or the destination guard (dst != IGMP_ALL_HOSTS && dst != group) survived the suite: the spoofed case silently armed a report timer, the later compliant case coalesced into it per the s5.2 pending-response rule, and the single final poll emitted exactly one report, satisfying the closing assertion. Each spoofed case now also asserts that no report timer was armed (s.mcast[i].tmr_report == NO_TIMER), then polls past the Max Resp window (10 s) and asserts that still nothing was sent. Cases run at distinct tick marks (t=0, 10001, 20001) so a report armed by a mutated guard cannot be hidden by the compliant case's coalescing. The compliant case now asserts the timer is armed and that exactly one report is emitted. Mutation-checked (make unit-multicast): deleting the TTL guard fails the case-1 NO_TIMER assert, deleting the destination guard fails the case-2 NO_TIMER assert, && -> || fails the compliant case (plus the existing refresh/flood tests), and != 1 -> == 1 fails case 1. --- src/test/unit/unit_tests_multicast.c | 46 ++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/src/test/unit/unit_tests_multicast.c b/src/test/unit/unit_tests_multicast.c index d06beed5..42dcf1bf 100644 --- a/src/test/unit/unit_tests_multicast.c +++ b/src/test/unit/unit_tests_multicast.c @@ -384,11 +384,22 @@ END_TEST * 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). */ + * group memberships). + * + * F-6475: the report is always deferred to a timer, so "no frame sent + * synchronously" proves nothing on its own - a deleted guard would still + * pass that assertion and only surface as a report on a later poll (hidden + * by the §5.2 coalescing of the compliant case). Each spoofed case here + * therefore also asserts that no report timer was armed, then polls past + * the Max Resp window and asserts that still nothing was sent. Cases run at + * distinct tick marks so a report armed by a mutated guard cannot be + * coalesced into or hidden by the compliant case. */ START_TEST(test_multicast_igmp_query_spoofed_dropped) { struct wolfIP s; int sd; + int m_idx = -1; + unsigned int i; struct wolfIP_ip_mreq mreq; struct wolfIP_ll_dev *ll; uint8_t frame[ETH_HEADER_LEN + IP_HEADER_LEN + IGMPV3_QUERY_MIN_LEN]; @@ -407,7 +418,15 @@ START_TEST(test_multicast_igmp_query_spoofed_dropped) 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. */ + for (i = 0; i < WOLFIP_MCAST_MEMBERSHIPS; i++) { + if (s.mcast[i].refs > 0 && s.mcast[i].group == group) + m_idx = (int)i; + } + ck_assert_int_gt(m_idx, -1); + + /* (1) Otherwise-valid general query but with TTL != 1 -> dropped: no + * synchronous frame and no deferred report armed. Polling past the Max + * Resp window (100 = 10 s) must still send nothing. */ 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); @@ -419,12 +438,17 @@ START_TEST(test_multicast_igmp_query_spoofed_dropped) ip->src = ee32(0x0A000001U); ip->dst = ee32(IGMP_ALL_HOSTS); igmp[0] = IGMP_TYPE_MEMBERSHIP_QUERY; + igmp[1] = 100; /* Max Resp Code 100 = 10 s */ 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; + last_frame_sent_count = 0; wolfIP_recv_ex(&s, TEST_PRIMARY_IF, frame, sizeof(frame)); ck_assert_uint_eq(last_frame_sent_size, 0); + ck_assert_uint_eq(s.mcast[m_idx].tmr_report, NO_TIMER); + wolfIP_poll(&s, 10001); + ck_assert_uint_eq(last_frame_sent_count, 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. */ @@ -439,16 +463,21 @@ START_TEST(test_multicast_igmp_query_spoofed_dropped) ip->src = ee32(0x0A000001U); ip->dst = ee32(0x0A000002U); /* our unicast IP */ igmp[0] = IGMP_TYPE_MEMBERSHIP_QUERY; + igmp[1] = 100; /* Max Resp Code 100 = 10 s */ 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; + last_frame_sent_count = 0; wolfIP_recv_ex(&s, TEST_PRIMARY_IF, frame, sizeof(frame)); ck_assert_uint_eq(last_frame_sent_size, 0); + ck_assert_uint_eq(s.mcast[m_idx].tmr_report, NO_TIMER); + wolfIP_poll(&s, 20001); + ck_assert_uint_eq(last_frame_sent_count, 0); - /* Sanity: a compliant query (TTL 1, all-hosts dst) still solicits a report - * (deferred per RFC 3376 §5.2, then emitted on poll), so the guards did not - * over-block. */ + /* (3) Sanity: a compliant query (TTL 1, all-hosts dst) still solicits a + * report (deferred per RFC 3376 §5.2, then emitted on poll), 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); @@ -460,14 +489,19 @@ START_TEST(test_multicast_igmp_query_spoofed_dropped) ip->src = ee32(0x0A000001U); ip->dst = ee32(IGMP_ALL_HOSTS); igmp[0] = IGMP_TYPE_MEMBERSHIP_QUERY; + igmp[1] = 100; /* Max Resp Code 100 = 10 s */ 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; + last_frame_sent_count = 0; wolfIP_recv_ex(&s, TEST_PRIMARY_IF, frame, sizeof(frame)); ck_assert_uint_eq(last_frame_sent_size, 0); - wolfIP_poll(&s, 2); + ck_assert_uint_ne(s.mcast[m_idx].tmr_report, NO_TIMER); + wolfIP_poll(&s, 30001); + ck_assert_uint_eq(last_frame_sent_count, 1); ck_assert_uint_gt(last_frame_sent_size, 0); + ck_assert_uint_eq(last_igmp_payload()[8], IGMPV3_REC_MODE_IS_EXCLUDE); } END_TEST From 9ffc67455de7cde00f8031b2e8843a4466f80701 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 13:37:23 +0200 Subject: [PATCH 06/14] F-8521: record the alignment-induced head wrap in fifo_push MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fifo_align_head_pos() wraps an unaligned head in {size-3, size-2, size-1} to 0, but unlike the explicit end-of-buffer branch it never records the wrap in h_wrap. When the FIFO is non-empty and h_wrap is 0, the collapsed head==tail==0 && h_wrap==0 state is indistinguishable from the empty state: the space test in fifo_push reports the whole buffer as free and the next push writes a fresh descriptor at offset 0, clobbering every previously queued descriptor (silent loss of UDP/ICMP/raw datagrams). A second variant — a wrap write ending exactly on tail — left a non-empty FIFO that reported empty and orphaned all live descriptors. Record the wrap (h_wrap = pre-alignment head) in fifo_push when alignment collapses a non-zero head to 0 on a non-empty, not-yet-wrapped FIFO, and mirror the same rule in fifo_can_push_len so upstream capacity checks agree with the fixed empty/full test. A rejected push mutates no state; a recorded h_wrap is cleared by the existing fifo_pop drain path. Regression tests: (1) a descriptor filling [0, size-2) with tail 0 must make the next push fail with -1 and leave the queued descriptor intact; (2) the wrap-lands-on-tail sequence must leave the FIFO reporting non-empty with the oldest live descriptor still reachable via fifo_peek. Both were verified to fail against the unfixed code (the push returned 0 and clobbered / the FIFO reported empty) and pass with the fix, across the plain, IP_MULTICAST and VLAN unit builds. --- src/test/unit/unit.c | 2 + src/test/unit/unit_tests_fifo.c | 101 ++++++++++++++++++++++++++++++++ src/wolfip.c | 28 +++++++-- 3 files changed, 127 insertions(+), 4 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 5c4c3c2b..071ddb52 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -68,6 +68,8 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_fifo_wrap_full_pop_then_refill_keeps_order_without_drops); tcase_add_test(tc_core, test_fifo_wrap_flag_transitions_push_pop_around_boundary); tcase_add_test(tc_core, test_fifo_wrap_flag_repeated_flips_keep_data_consistent); + tcase_add_test(tc_core, test_fifo_push_align_wrap_tail0_rejects_not_clobbers); + tcase_add_test(tc_core, test_fifo_push_align_wrap_keeps_nonempty_state); tcase_add_test(tc_core, test_fifo_wrap_flag_transitions_with_odd_payload_sizes); tcase_add_test(tc_core, test_fifo_wrap_flag_repeated_flips_with_odd_payload_sizes); diff --git a/src/test/unit/unit_tests_fifo.c b/src/test/unit/unit_tests_fifo.c index 351a9ba2..5b66156d 100644 --- a/src/test/unit/unit_tests_fifo.c +++ b/src/test/unit/unit_tests_fifo.c @@ -631,3 +631,104 @@ START_TEST(test_queue_insert_no_head_update_when_pos_plus_len_le_head) } END_TEST + +/* F-8521: a single descriptor that fills the ring just short of the end + * (head left unaligned at size-2) makes fifo_align_head_pos() wrap the + * insertion cursor to 0. If that wrap is not recorded in h_wrap, the + * nearly-full FIFO is indistinguishable from the empty state, the space + * test reports the whole buffer as free, and the next push clobbers the + * live descriptor at offset 0 instead of being rejected for lack of space. */ +START_TEST(test_fifo_push_align_wrap_tail0_rejects_not_clobbers) +{ + struct fifo f; + uint8_t data[64]; + uint8_t big[46]; + uint8_t small[8]; + struct pkt_desc *desc; + int i; + + memset(data, 0, sizeof(data)); + for (i = 0; i < 46; i++) + big[i] = (uint8_t)(0x10 + i); + for (i = 0; i < 8; i++) + small[i] = (uint8_t)(0x50 + i); + + fifo_init(&f, data, sizeof(data)); + + /* One descriptor filling [0,62): 16-byte pkt_desc + 46 payload. + * head = 62 (unaligned), tail = 0, h_wrap = 0, non-empty. */ + ck_assert_int_eq(fifo_push(&f, big, sizeof(big)), 0); + ck_assert_uint_eq(f.head, 62); + ck_assert_uint_eq(f.tail, 0); + ck_assert_uint_eq(f.h_wrap, 0); + ck_assert_int_eq(fifo_is_empty(&f), 0); + + /* Second push: aligned head 62 -> 64 -> 0. The wrap must be recorded so + * the (nearly full) FIFO is not mistaken for empty; the push is rejected + * for lack of space rather than overwriting the live descriptor. */ + ck_assert_int_eq(fifo_push(&f, small, sizeof(small)), -1); + + /* The queued descriptor must survive intact. */ + ck_assert_int_eq(fifo_is_empty(&f), 0); + ck_assert_uint_eq(f.head, 62); + desc = fifo_peek(&f); + ck_assert_ptr_nonnull(desc); + ck_assert_uint_eq(desc->pos, 0); + ck_assert_uint_eq(desc->len, 46); + ck_assert_mem_eq(data + 16, big, 46); +} +END_TEST + +/* F-8521 (wrap-lands-on-tail variant): a wrap write whose aligned head + * collapses to 0 and whose payload ends exactly on tail stores head == tail. + * Without the h_wrap marker the non-empty FIFO reports as empty and every + * previously queued descriptor is orphaned (fifo_peek returns NULL). The + * wrap must be recorded so the FIFO stays visible and the oldest live + * descriptor remains reachable. */ +START_TEST(test_fifo_push_align_wrap_keeps_nonempty_state) +{ + struct fifo f; + uint8_t data[64]; + uint8_t p0[8], pj[4], pk[1], trig[8]; + struct pkt_desc *desc; + int i; + + memset(data, 0, sizeof(data)); + for (i = 0; i < 8; i++) + p0[i] = (uint8_t)(0xA0 + i); + for (i = 0; i < 4; i++) + pj[i] = (uint8_t)(0xB0 + i); + pk[0] = 0xC0; + for (i = 0; i < 8; i++) + trig[i] = (uint8_t)(0xD0 + i); + + fifo_init(&f, data, sizeof(data)); + + /* p0@0 (head 24), pj@24 (head 44), pk@44 (head 61, unaligned). */ + ck_assert_int_eq(fifo_push(&f, p0, sizeof(p0)), 0); + ck_assert_int_eq(fifo_push(&f, pj, sizeof(pj)), 0); + ck_assert_int_eq(fifo_push(&f, pk, sizeof(pk)), 0); + ck_assert_uint_eq(f.head, 61); + ck_assert_uint_eq(f.h_wrap, 0); + + /* Pop p0: tail 0 -> 24. Two descriptors live at 24 and 44. */ + desc = fifo_pop(&f); + ck_assert_ptr_nonnull(desc); + ck_assert_uint_eq(f.tail, 24); + ck_assert_int_eq(fifo_is_empty(&f), 0); + + /* Trigger: needed 24 == tail. Aligned head 61 -> 64 -> 0; the wrap write + * [0,24) ends exactly on tail. The wrap must be recorded. */ + ck_assert_int_eq(fifo_push(&f, trig, sizeof(trig)), 0); + ck_assert_uint_eq(f.head, f.tail); + + /* Three descriptors are live: the FIFO must report non-empty and peek + * must reach the oldest live one (pj at 24). */ + ck_assert_int_eq(fifo_is_empty(&f), 0); + desc = fifo_peek(&f); + ck_assert_ptr_nonnull(desc); + ck_assert_uint_eq(desc->pos, 24); + ck_assert_uint_eq(desc->len, 4); + ck_assert_mem_eq(data + 40, pj, 4); +} +END_TEST diff --git a/src/wolfip.c b/src/wolfip.c index de808b87..0e2d8d5c 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -372,7 +372,19 @@ static int fifo_push(struct fifo *f, void *data, uint32_t len) uint32_t h_wrap = f->h_wrap; memset(&desc, 0, sizeof(struct pkt_desc)); /* Ensure 4-byte alignment in the buffer */ - head = fifo_align_head_pos(head, f->size); + { + uint32_t raw_head = head; + head = fifo_align_head_pos(head, f->size); + /* fifo_align_head_pos() wraps an unaligned head in {size-3,size-2, + * size-1} to 0. If the FIFO is non-empty and not yet wrapped, that + * wrap must be recorded in h_wrap: otherwise head==tail==0 && + * h_wrap==0 is indistinguishable from the empty state, the space test + * below reports the whole buffer as free, and the push clobbers every + * previously queued descriptor (or, when the write lands exactly on + * tail, leaves a non-empty FIFO that reports as empty). */ + if (head == 0 && raw_head != 0 && h_wrap == 0 && !fifo_is_empty(f)) + h_wrap = raw_head; + } { uint32_t space; if (head == tail && h_wrap == 0) @@ -440,9 +452,17 @@ static int fifo_can_push_len(const struct fifo *fin, uint32_t len) needed = sizeof(struct pkt_desc) + len; if (needed > fin->size) return 0; - head = fifo_align_head_pos(fin->head, fin->size); - tail = fin->tail; - h_wrap = fin->h_wrap; + { + uint32_t raw_head = fin->head; + head = fifo_align_head_pos(fin->head, fin->size); + tail = fin->tail; + h_wrap = fin->h_wrap; + /* Mirror fifo_push(): record an alignment-induced head->0 wrap so the + * capacity check agrees with the empty/full test rather than reporting + * a nearly-full FIFO as fully free. */ + if (head == 0 && raw_head != 0 && h_wrap == 0 && !fifo_is_empty(fin)) + h_wrap = raw_head; + } { uint32_t space; From c268ae17ce9e70590bae429354a2a06abb7eff50 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 13:52:56 +0200 Subject: [PATCH 07/14] F-10280: scope the udp_try_recv DHCP relaxation to the DHCP socket The addr_match expression relaxed all peer and destination validation whenever a socket had no local address (local_ip == 0) while the DHCP state machine was running: ((t->local_ip == 0) && DHCP_IS_RUNNING(s)). That relaxation exists so the DHCP client socket can receive OFFER/ACK before it owns an address, but as written it applied to every socket in s->udpsockets[] - so a connected application socket created before the interface had an address accepted datagrams from any source address and port for as long as local_ip stayed 0 (initial acquisition and every RENEWING/REBINDING cycle), bypassing the connected-peer filter. Compute is_dhcp (the socket's fd equals s->dhcp_udp_sd) and require it in the relaxation clause, keeping peer_match in force for every other socket regardless of local_ip. The DHCP socket is unconnected (peer_match is already 1) and keeps local_ip 0, so OFFER/ACK delivery is unchanged. Adds test_udp_dhcp_relaxation_scoped_to_dhcp_socket: a connected socket with local_ip 0 must not receive a datagram from a non-connected peer while DHCP runs, and the DHCP socket must still receive one from any source. Verified RED (the app socket received the spoofed datagram pre-fix) and GREEN. test_udp_try_recv_dhcp_running_local_zero, which codified the old over-broad relaxation on a non-DHCP socket, now marks its socket as the DHCP socket to keep asserting the intended relaxation. Plain, IP_MULTICAST and VLAN unit builds pass. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_dns_dhcp.c | 4 ++ src/test/unit/unit_tests_proto.c | 76 +++++++++++++++++++++++++++++ src/wolfip.c | 9 +++- 4 files changed, 89 insertions(+), 1 deletion(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 071ddb52..e96a8014 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -978,6 +978,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_regression_udp_len_exceeds_ip_len_dropped); tcase_add_test(tc_proto, test_regression_udp_len_below_header_discards_and_unblocks); tcase_add_test(tc_proto, test_regression_udp_payload_exceeds_buffer_discards_and_unblocks); + tcase_add_test(tc_proto, test_udp_dhcp_relaxation_scoped_to_dhcp_socket); tcase_add_test(tc_proto, test_regression_icmp_payload_exceeds_buffer_discards_and_unblocks); tcase_add_test(tc_proto, test_regression_tcp_ip_len_below_ip_header); tcase_add_test(tc_proto, test_regression_syn_on_established_not_silently_processed); diff --git a/src/test/unit/unit_tests_dns_dhcp.c b/src/test/unit/unit_tests_dns_dhcp.c index 437d095a..7acc9d25 100644 --- a/src/test/unit/unit_tests_dns_dhcp.c +++ b/src/test/unit/unit_tests_dns_dhcp.c @@ -6457,6 +6457,10 @@ START_TEST(test_udp_try_recv_dhcp_running_local_zero) ck_assert_ptr_nonnull(ts); ts->src_port = 1234; ts->local_ip = 0; + /* F-10280: the local_ip==0 relaxation is scoped to the DHCP client + * socket, so this socket must be the DHCP socket to receive before it + * owns an address. */ + s.dhcp_udp_sd = (int)(MARK_UDP_SOCKET | (uint32_t)(ts - s.udpsockets)); memset(udp_buf, 0, sizeof(udp_buf)); udp->ip.dst = ee32(local_ip); diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index 18fa8564..313b85a3 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -5601,6 +5601,82 @@ START_TEST(test_regression_icmp_payload_exceeds_buffer_discards_and_unblocks) } END_TEST +/* F-10280: while the DHCP client is running, the local_ip==0 relaxation in + * udp_try_recv must apply only to the DHCP client socket. A connected app + * socket created before the interface owns an address (local_ip==0) must + * still enforce peer matching, so a datagram from a non-connected peer is + * not delivered to it; the DHCP socket itself must still receive datagrams + * from any source. Socket fields are host order; wire fields are network + * order (ee16/ee32), matching udp_try_recv's comparison convention. */ +START_TEST(test_udp_dhcp_relaxation_scoped_to_dhcp_socket) +{ + struct wolfIP s; + struct tsocket *app; + struct tsocket *dhc; + uint8_t buf[sizeof(struct wolfIP_udp_datagram) + 32]; + struct wolfIP_udp_datagram *udp = (struct wolfIP_udp_datagram *)buf; + uint8_t payload[8]; + uint16_t total; + + wolfIP_init(&s); + mock_link_init(&s); + /* No interface IP is configured: sockets created now carry local_ip 0. + * The DHCP client is running. */ + s.dhcp_state = DHCP_RENEWING; + + /* App socket: connected to 10.0.0.2:9001, local port 9000. */ + app = udp_new_socket(&s); + ck_assert_ptr_nonnull(app); + app->src_port = 9000; + app->local_ip = 0; + app->remote_ip = 0x0A000002U; + app->dst_port = 9001; + app->sock.udp.connected = 1; + + /* DHCP client socket: unconnected, local port 68, local_ip 0. */ + dhc = udp_new_socket(&s); + ck_assert_ptr_nonnull(dhc); + dhc->src_port = 68; + dhc->local_ip = 0; + dhc->sock.udp.connected = 0; + s.dhcp_udp_sd = (int)(MARK_UDP_SOCKET | (uint32_t)(dhc - s.udpsockets)); + + memset(payload, 0x5A, sizeof(payload)); + total = UDP_HEADER_LEN + sizeof(payload); + + /* (1) Datagram to the app socket's port from a non-connected peer + * (10.0.0.99:1234, not the connected 10.0.0.2:9001). peer_match must + * reject it: the relaxation is scoped to the DHCP socket. */ + memset(buf, 0, sizeof(buf)); + udp->src_port = ee16(1234); + udp->dst_port = ee16(9000); + udp->len = ee16(total); + udp->csum = 0; /* skip checksum validation */ + udp->ip.len = ee16(IP_HEADER_LEN + total); + udp->ip.src = ee32(0x0A000099U); + udp->ip.dst = ee32(0x0A000002U); + memcpy(udp->data, payload, sizeof(payload)); + udp_try_recv(&s, TEST_PRIMARY_IF, udp, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + total)); + ck_assert_ptr_eq(fifo_peek(&app->sock.udp.rxbuf), NULL); + + /* (2) Datagram to the DHCP socket's port from any source: the relaxation + * still applies to the DHCP socket, so it must be delivered. */ + memset(buf, 0, sizeof(buf)); + udp->src_port = ee16(67); + udp->dst_port = ee16(68); + udp->len = ee16(total); + udp->csum = 0; + udp->ip.len = ee16(IP_HEADER_LEN + total); + udp->ip.src = ee32(0x0A000099U); + udp->ip.dst = ee32(0xFFFFFF00U); /* broadcast */ + memcpy(udp->data, payload, sizeof(payload)); + udp_try_recv(&s, TEST_PRIMARY_IF, udp, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + total)); + ck_assert_ptr_nonnull(fifo_peek(&dhc->sock.udp.rxbuf)); +} +END_TEST + START_TEST(test_regression_icmp_ip_len_below_header) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 0e2d8d5c..06b6f432 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -2737,8 +2737,15 @@ static void udp_try_recv(struct wolfIP *s, unsigned int if_idx, int peer_match = (t->sock.udp.connected == 0) || ((t->dst_port == 0 || t->dst_port == ee16(udp->src_port)) && (t->remote_ip == 0 || t->remote_ip == src_ip)); + /* The local_ip==0 relaxation exists so the DHCP client socket can + * receive OFFER/ACK before it owns an address. It must apply only to + * that socket: scoping it to s->dhcp_udp_sd keeps peer_match in force + * for any other (e.g. connected) socket that still has no local + * address while DHCP is running. */ + int is_dhcp = (s->dhcp_udp_sd > 0) && + ((uint32_t)(MARK_UDP_SOCKET | i) == (uint32_t)s->dhcp_udp_sd); int addr_match = - (((t->local_ip == 0) && DHCP_IS_RUNNING(s)) || + (((t->local_ip == 0) && DHCP_IS_RUNNING(s) && is_dhcp) || (t->local_ip == dst_ip && peer_match)); #ifdef IP_MULTICAST if (wolfIP_ip_is_multicast(dst_ip)) { From e275c3528c8f292a32b6ea28661a05a028cf239a Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 14:16:24 +0200 Subject: [PATCH 08/14] F-10281: validate bound_local_ip for non-SYN segments to a bound listener In tcp_input the per-socket match gated the 4-tuple comparison on state > TCP_LISTEN, so a TCP_LISTEN socket was matched on local port alone. The SYN handler separately validates bound_local_ip, but a non-SYN segment (data/ACK/FIN) for the same port and a different local address on the same host fell through to the shared bookkeeping writes - t->if_idx, t->last_pkt_ttl, matched = 1, and (for non-RST) t->sock. tcp.peer_rwnd - before any destination-address check. That lets a peer that only needs to know a specifically-bound listener's port corrupt the listener's MTU/TTL bookkeeping, seed the initial window of a later accepted child, and set matched so the RFC 793 unmatched-segment RST is suppressed. Add an else branch so a listener in TCP_LISTEN is skipped when its bound_local_ip is a specific address that does not equal the segment's destination. bound_local_ip (not local_ip) is the discriminator: a 0.0.0.0 bind leaves local_ip set to the interface/primary address as a default source, so local_ip cannot tell a wildcard listener from a specifically-bound one. Wildcard listeners are unaffected (no-op), and the established-socket path (state > TCP_LISTEN) is unchanged. Note on the finding's RST-blackhole claim: a TCP_CLOSED socket with a non-zero src_port never reaches the port-match gate - close_socket() and every RX-path teardown zero proto (first guard) or set CB_EVENT_CLOSED (second guard) and always zero src_port, so a matchable TCP_CLOSED slot only matches invalid port-0 segments. That part of the report does not manifest; the local-address validation gap for listeners is real and is what this commit closes. Adds test_tcp_listen_requires_matching_local_ip: a non-SYN segment for a bound listener's port but a different local address must not change last_pkt_ttl, while one for the bound address still matches. Verified RED (last_pkt_ttl became 64 pre-fix) and GREEN. Plain, IP_MULTICAST and VLAN unit builds pass. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_proto.c | 47 ++++++++++++++++++++++++++++++++ src/wolfip.c | 17 ++++++++++++ 3 files changed, 65 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index e96a8014..c6cafbd7 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -925,6 +925,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_regression_icmp_echo_request_non_local_dst_no_reply); tcase_add_test(tc_proto, test_tcp_listen_rejects_wrong_interface); tcase_add_test(tc_proto, test_tcp_listen_accepts_bound_interface); + tcase_add_test(tc_proto, test_tcp_listen_requires_matching_local_ip); tcase_add_test(tc_proto, test_tcp_listen_accepts_any_interface); tcase_add_test(tc_proto, test_sock_connect_selects_local_ip_multi_if); tcase_add_test(tc_proto, test_icmp_socket_send_recv); diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index 313b85a3..78576c53 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -4811,6 +4811,53 @@ START_TEST(test_tcp_listen_accepts_bound_interface) } END_TEST +/* F-10281: a listener bound to a specific local address must only match + * segments addressed to that address. The SYN path already validates + * bound_local_ip, but a non-SYN segment for the same port and a different + * local address on the same host overwrites the listener's + * if_idx/last_pkt_ttl/peer_rwnd and sets matched (suppressing the RFC 793 + * unmatched RST) before any address validation. A segment for the bound + * address must still match, so the check is not over-restricting. */ +START_TEST(test_tcp_listen_requires_matching_local_ip) +{ + struct wolfIP s; + const ip4 primary_ip = 0xC0A80002U; + const ip4 secondary_ip = 0xC0A80101U; + const uint16_t listen_port = 23456; + int listen_fd; + struct wolfIP_sockaddr_in addr; + struct tsocket *listener; + uint8_t ttl_before; + + setup_stack_with_two_ifaces(&s, primary_ip, secondary_ip); + + listen_fd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_STREAM, 0); + ck_assert_int_ge(listen_fd, 0); + listener = &s.tcpsockets[SOCKET_UNMARK(listen_fd)]; + + memset(&addr, 0, sizeof(addr)); + addr.sin_family = AF_INET; + addr.sin_port = ee16(listen_port); + addr.sin_addr.s_addr = ee32(secondary_ip); + ck_assert_int_eq(wolfIP_sock_bind(&s, listen_fd, (struct wolfIP_sockaddr *)&addr, sizeof(addr)), 0); + ck_assert_int_eq(wolfIP_sock_listen(&s, listen_fd, 1), 0); + ck_assert_uint_eq(listener->bound_local_ip, secondary_ip); + ck_assert_int_eq(listener->sock.tcp.state, TCP_LISTEN); + ttl_before = listener->last_pkt_ttl; + + /* (1) A non-SYN segment for the same port but a different local address + * must not mutate the listener's bookkeeping. */ + inject_tcp_segment(&s, TEST_PRIMARY_IF, 0x0A0000A1U, primary_ip, 40000, + listen_port, 100, 0, 0); + ck_assert_uint_eq(listener->last_pkt_ttl, ttl_before); + + /* (2) A segment for the bound address still matches (not over-restricted). */ + inject_tcp_segment(&s, TEST_SECOND_IF, 0x0A0000A2U, secondary_ip, 40000, + listen_port, 200, 0, 0); + ck_assert_uint_eq(listener->last_pkt_ttl, 64); +} +END_TEST + START_TEST(test_tcp_listen_accepts_any_interface) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 06b6f432..2e5f02b5 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -5460,6 +5460,23 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, /* Not the right local endpoint */ continue; } + } else { + /* LISTEN: a specifically-bound listener (bound_local_ip != + * 0.0.0.0) must only match segments addressed to its bound + * address; a wildcard listener accepts any local address. + * The SYN path already enforces this for SYNs, but without it + * here a non-SYN segment for a different local IP on the same + * host overwrites the listener's if_idx/last_pkt_ttl/peer_rwnd + * and sets matched, which corrupts listener MTU/TTL + * bookkeeping and suppresses the RFC 793 unmatched RST. + * bound_local_ip (not local_ip) is the discriminator: a + * 0.0.0.0 bind leaves local_ip set to the interface/primary + * address as a default source. */ + if (t->bound_local_ip != IPADDR_ANY && + t->bound_local_ip != ee32(tcp->ip.dst)) { + /* Not the right local endpoint */ + continue; + } } t->if_idx = (uint8_t)if_idx; t->last_pkt_ttl = tcp->ip.ttl; From ead1ca013a40993f17779339c649273f21678578 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 14:18:33 +0200 Subject: [PATCH 09/14] F-6471: remove dead UDP datagram length re-check in udp_try_recv The per-socket match loop recomputed expected_len = ee16(udp->len) + IP_HEADER_LEN + ETH_HEADER_LEN and bailed with if ((int)frame_len < (int)expected_len) return;. That branch is unreachable: the unconditional guard before the socket loop already rejects any datagram where ee16(udp->len) > frame_len - ETH_HEADER_LEN - IP_HEADER_LEN, i.e. it guarantees frame_len >= expected_len. frame_len is pass-by-value and udp->len is not modified between the two, so the inner comparison is always false for every caller (the dispatch path presents an option-stripped IHL=5 header; the loopback multicast caller builds frames where the two quantities are exactly equal). Remove the dead comparison and its now-unused expected_len local. No behavioral change; the plain, IP_MULTICAST and VLAN unit suites pass unchanged. --- src/wolfip.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/wolfip.c b/src/wolfip.c index 2e5f02b5..72de5a8f 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -2727,7 +2727,6 @@ static void udp_try_recv(struct wolfIP *s, unsigned int if_idx, return; for (i = 0; i < MAX_UDPSOCKETS; i++) { struct tsocket *t = &s->udpsockets[i]; - uint32_t expected_len; /* Only connected UDP sockets restrict by the peer's * ip/port. Unconnected sockets (sendto-only or pure listeners) * must accept datagrams from any source, per POSIX. This is @@ -2760,13 +2759,10 @@ static void udp_try_recv(struct wolfIP *s, unsigned int if_idx, if (t->local_ip == 0) t->if_idx = (uint8_t)if_idx; - /* UDP datagram sanity checks */ - /* Allow some tolerance for padding/alignment (up to 4 bytes) */ - expected_len = ee16(udp->len) + IP_HEADER_LEN + ETH_HEADER_LEN; - if ((int)frame_len < (int)expected_len) - return; /* A bound socket matched this datagram. If the RX FIFO is full, - * drop silently instead of misreporting the port as closed. */ + * drop silently instead of misreporting the port as closed. + * (The frame_len vs declared UDP length bound is already + * enforced by the unconditional guard before the socket loop.) */ matched = 1; if (fifo_push(&t->sock.udp.rxbuf, udp, frame_len) == 0) { t->last_pkt_ttl = udp->ip.ttl; From 49cedb0288a7b1aeeb20d1c230c623aa5e8feb66 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 14:19:24 +0200 Subject: [PATCH 10/14] F-9374: document ssh_server_get_uptime as a placeholder The header described ssh_server_get_uptime as returning the SSH server uptime in seconds, but the implementation returns the constant 0 until a main-loop tick source is integrated, so the "uptime" SSH command always reports zero. Correct the API-contract comment to state that it is a placeholder returning 0. No behavioral change. --- src/port/stm32h563/ssh_server.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/port/stm32h563/ssh_server.h b/src/port/stm32h563/ssh_server.h index 1a77085c..b33231e4 100644 --- a/src/port/stm32h563/ssh_server.h +++ b/src/port/stm32h563/ssh_server.h @@ -35,7 +35,9 @@ int ssh_server_init(struct wolfIP *stack, uint16_t port, ssh_debug_cb debug); * Returns 0 on success */ int ssh_server_poll(void); -/* Get SSH server uptime in seconds (for status display) */ +/* Get SSH server uptime in seconds (for status display). + * Currently a placeholder: returns 0 until a main-loop tick source is + * integrated, so the "uptime" SSH command always reports zero. */ uint32_t ssh_server_get_uptime(void); #endif /* SSH_SERVER_H */ From c10690dccd54d8650f53bcc0e627b6be81c630d5 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 14:20:43 +0200 Subject: [PATCH 11/14] F-9380: correct TFTP worst-case request-size comment for timeout The WOLFTFTP_REQ_BUF_MAX comment totaled the worst-case RRQ/WRQ as 63 + MAX_FILENAME, assigning 12 bytes to the timeout option. But timeout_s is an unclamped uint16_t serialized by wolftftp_append_opt, which includes both key and value terminators: 65535 becomes "timeout\0" (8) + "65535\0" (6) = 14 bytes. The correct total is 65 + MAX_FILENAME. The WOLFTFTP_REQ_BUF_MAX allocation (MAX_FILENAME + 128) already covers this with margin to spare, so no behavioral change - the fix stops a maintainer from using the documented calculation to reduce the margin below a valid maximal request. --- src/tftp/wolftftp.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/tftp/wolftftp.h b/src/tftp/wolftftp.h index 085efea7..5397ff15 100644 --- a/src/tftp/wolftftp.h +++ b/src/tftp/wolftftp.h @@ -67,9 +67,11 @@ /* Worst-case RRQ/WRQ on the wire: * opcode(2) + filename(MAX_FILENAME, null-terminated) + "octet\0"(6) - * + blksize/value(13) + timeout/value(12) + windowsize/value(13) - * + tsize/value(17) = 63 + MAX_FILENAME. The constant below adds a - * generous margin so future options do not silently truncate. */ + * + blksize/value(13) + timeout/value(14) + windowsize/value(13) + * + tsize/value(17) = 65 + MAX_FILENAME. The constant below adds a + * generous margin so future options do not silently truncate. The timeout + * value is the widest: timeout_s is an unclamped uint16_t, so 65535 serializes + * as "65535\0" (6 bytes) behind "timeout\0" (8 bytes). */ #define WOLFTFTP_REQ_BUF_MAX (WOLFTFTP_MAX_FILENAME + 128U) #define WOLFTFTP_ERR_IO (-1000) From 2ad91d7528bd707a050311c360db64698d86da6f Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 14:25:36 +0200 Subject: [PATCH 12/14] F-6211: detect DNS responses by the QR bit alone dns_callback only parsed an incoming datagram as a DNS response when (flags & DNS_FLAGS_RESPONSE_RD) == DNS_FLAGS_RESPONSE_RD, i.e. when both the QR (query/response) and RD (recursion-desired) bits were set (0x8100). Per RFC 1035 s4.1.1 the QR bit alone distinguishes a response from a query; RD is merely the Recursion-Desired flag a server echoes from the query. A conformant server that does not echo RD (a response otherwise lacking the RD bit) therefore had its reply silently ignored, letting the outstanding query time out and retransmit needlessly. Add a DNS_FLAGS_RESPONSE macro for the QR bit and gate response parsing on it. DNS_FLAGS_RESPONSE_RD is retained: it is still a valid QR|RD response flags value used by the test helpers to build conformant replies. Adds test_dns_callback_qr_without_rd_is_accepted: a response with QR set and RD clear must be parsed and the lookup delivered. Verified RED (lookup not delivered pre-fix) and GREEN. Plain, IP_MULTICAST and VLAN unit builds pass. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_dns_edges.c | 51 ++++++++++++++++++++++++++++ src/wolfip.c | 13 +++++-- 3 files changed, 62 insertions(+), 3 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index c6cafbd7..b6337e73 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -1537,6 +1537,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_dns_callback_rcode_nonzero_aborts_query); tcase_add_test(tc_core, test_dns_callback_zero_ancount_no_delivery); tcase_add_test(tc_core, test_dns_callback_aaaa_answer_skipped_for_a_query); + tcase_add_test(tc_core, test_dns_callback_qr_without_rd_is_accepted); tcase_add_test(tc_core, test_dns_callback_rr_rdlen_truncated_aborts_query); tcase_add_test(tc_core, test_dns_callback_bad_question_name_aborts_query); tcase_add_test(tc_core, test_dns_callback_answer_forward_ptr_aborts_query); diff --git a/src/test/unit/unit_tests_dns_edges.c b/src/test/unit/unit_tests_dns_edges.c index 97619ef6..c17c23ba 100644 --- a/src/test/unit/unit_tests_dns_edges.c +++ b/src/test/unit/unit_tests_dns_edges.c @@ -196,6 +196,57 @@ START_TEST(test_dns_callback_aaaa_answer_skipped_for_a_query) } END_TEST +/* ------------------------------------------------------------------ * + * F-6211: response detection must key on the QR bit alone (RFC 1035 + * s4.1.1). A conformant server that does not echo the RD bit must still + * have its reply parsed. Requiring RD as well silently drops such a + * response and lets the query time out and retransmit. Flags here are + * 0x8000 (QR set, RD clear). + * ------------------------------------------------------------------ */ +START_TEST(test_dns_callback_qr_without_rd_is_accepted) +{ + struct wolfIP s; + uint8_t response[128]; + int pos; + struct dns_rr *rr; + uint8_t a_rdata[4] = {0x0A, 0x00, 0x00, 0x02}; + + wolfIP_init(&s); + mock_link_init(&s); + s.dns_server = 0x0A000001U; + arm_dns_query(&s, 0x3333, dns_qname_example_com, + (int)sizeof(dns_qname_example_com), DNS_A); + dns_lookup_calls = 0; + dns_lookup_ip = 0; + s.dns_lookup_cb = test_dns_lookup_cb; + s.dns_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); + ck_assert_int_gt(s.dns_udp_sd, 0); + + /* QR set, RD clear, RCODE 0, TC clear. */ + pos = build_dns_a_response_header(response, sizeof(response), + s.dns_id, 0x8000, 1, 1, NULL); + /* Answer NAME: compressed pointer to the question name. */ + 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((uint16_t)sizeof(a_rdata)); + pos += (int)sizeof(struct dns_rr); + memcpy(&response[pos], a_rdata, sizeof(a_rdata)); + pos += (int)sizeof(a_rdata); + + 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); + + /* The QR-only response must be parsed and the lookup delivered. */ + ck_assert_int_eq(dns_lookup_calls, 1); + ck_assert_uint_eq(dns_lookup_ip, 0x0A000002U); +} +END_TEST + /* ------------------------------------------------------------------ * * dns_callback: answer rdlen advertised larger than remaining buffer * → abort query (line 8997-8999) diff --git a/src/wolfip.c b/src/wolfip.c index 72de5a8f..2e2230ba 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -10656,7 +10656,11 @@ void wolfIP_recv_ex(struct wolfIP *s, unsigned int if_idx, void *buf, uint32_t l #define DNS_RD 0x0100 /* Recursion desired */ #define DNS_TC 0x0200 /* Truncated response */ #define DNS_RCODE_MASK 0x000F -#define DNS_FLAGS_RESPONSE_RD (DNS_RD | ((uint16_t)DNS_RESPONSE << 8)) +/* QR bit (bit 15 of the 16-bit flags field): per RFC 1035 s4.1.1 this alone + * distinguishes a response from a query. RD is only the Recursion-Desired + * flag echoed from the query, so it must not gate response detection. */ +#define DNS_FLAGS_RESPONSE ((uint16_t)DNS_RESPONSE << 8) +#define DNS_FLAGS_RESPONSE_RD (DNS_RD | DNS_FLAGS_RESPONSE) #define DNS_ID_NONE 0 #define DNS_QUESTION_COUNT 1 #define DNS_MIN_ID 1 @@ -11009,8 +11013,11 @@ void dns_callback(int dns_sd, uint16_t ev, void *arg) if (ee16(hdr->id) != s->dns_id) return; flags = ee16(hdr->flags); - /* Parse DNS response */ - if ((flags & DNS_FLAGS_RESPONSE_RD) == DNS_FLAGS_RESPONSE_RD) { + /* Parse DNS response: key on the QR bit alone (RFC 1035 s4.1.1). A + * conformant server that does not echo the RD bit must still have its + * reply parsed; requiring RD as well silently drops such responses + * and lets the outstanding query time out and retransmit. */ + if ((flags & DNS_FLAGS_RESPONSE) != 0) { if ((flags & DNS_TC) != 0) { dns_abort_query(s); return; From f0e57b2793748b55f2e9831d5cf357ede10df5ee Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 14:35:03 +0200 Subject: [PATCH 13/14] F-8523: key the handle_socket_callbacks reap on socket identity After dispatching a TCP socket callback, handle_socket_callbacks() re-read ts->sock.tcp.state and, if TCP_CLOSED, disarmed the callback and called close_socket(), which memsets the whole slot. ts is a fixed slot address, so that check is purely positional. A close callback that closes the socket via wolfIP_sock_close() (freeing the slot) and then allocates a fresh one - tcp_new_socket() scans from index 0 for the first proto == 0 slot, so it can land in the very same slot - leaves the slot holding a brand-new socket whose state is TCP_CLOSED by design. The positional reap then destroyed that fresh socket (zeroing S, proto and the FIFO backing), leaving the application with a permanently dead descriptor. Capture the callback/callback_arg pair before invoking the callback and only reap if both are unchanged afterward: a replaced slot carries a different (or no) callback, so the reap no longer touches it. The normal deferred-close path is unaffected - there the callback does not replace the socket, so the pair is unchanged and the reap proceeds as before. A socket the callback closed without re-creating is already memset (callback NULL), so it is likewise left alone (close_socket would be a no-op). Adds test_handle_socket_callbacks_keeps_recreated_socket: a socket left in the RX-deferred close state (TCP_CLOSED + CB_EVENT_CLOSED) whose callback closes it and re-creates a socket in the same slot must survive the dispatcher's post-callback reap. Verified RED (slot proto zeroed pre-fix) and GREEN. Plain, IP_MULTICAST and VLAN unit builds pass. --- src/test/unit/unit.c | 1 + src/test/unit/unit_tests_tcp_ack.c | 54 ++++++++++++++++++++++++++++++ src/wolfip.c | 34 ++++++++++++------- 3 files changed, 77 insertions(+), 12 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index b6337e73..ac160a64 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -651,6 +651,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_tcp_fin_wait_1_to_closing); tcase_add_test(tc_utils, test_tcp_last_ack_closes_socket); tcase_add_test(tc_utils, test_tcp_last_ack_closes_socket_delivers_closed_event); + tcase_add_test(tc_utils, test_handle_socket_callbacks_keeps_recreated_socket); tcase_add_test(tc_utils, test_tcp_last_ack_partial_ack_keeps_socket_and_timer); tcase_add_test(tc_utils, test_tcp_ack_acks_data_and_sets_writable); tcase_add_test(tc_utils, test_tcp_ack_duplicate_resend_clears_sent); diff --git a/src/test/unit/unit_tests_tcp_ack.c b/src/test/unit/unit_tests_tcp_ack.c index 1e749ec5..c4b529da 100644 --- a/src/test/unit/unit_tests_tcp_ack.c +++ b/src/test/unit/unit_tests_tcp_ack.c @@ -4029,6 +4029,60 @@ START_TEST(test_tcp_last_ack_closes_socket_delivers_closed_event) } END_TEST +/* F-8523: the post-callback reap in handle_socket_callbacks must not destroy + * a socket that the close callback closed and re-created in the same slot. + * The reap historically keyed on the slot's TCP_CLOSED state, which a fresh + * socket has by design, so it must be gated on the slot still holding the + * dispatched socket (same callback pair). */ +static int test_f8523_recreated_fd; +static void test_f8523_close_recreate_cb(int fd, uint16_t events, void *arg) +{ + struct wolfIP *s = (struct wolfIP *)arg; + (void)events; + /* Close the socket (frees its slot) and create a fresh one in its place. */ + (void)wolfIP_sock_close(s, fd); + test_f8523_recreated_fd = wolfIP_sock_socket(s, AF_INET, IPSTACK_SOCK_STREAM, 0); +} + +START_TEST(test_handle_socket_callbacks_keeps_recreated_socket) +{ + struct wolfIP s; + struct tsocket *ts; + ip4 local_ip = 0x0A000001U; + uint16_t local_port = 6669; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, local_ip, 0xFFFFFF00U, 0); + + /* Put slot 0 in the RX-deferred close state: TCP_CLOSED with a pending + * CB_EVENT_CLOSED and an armed callback, no close_notify_pending. */ + ts = &s.tcpsockets[0]; + memset(ts, 0, sizeof(*ts)); + ts->proto = WI_IPPROTO_TCP; + ts->S = &s; + ts->sock.tcp.state = TCP_CLOSED; + ts->local_ip = local_ip; + ts->src_port = local_port; + ts->callback = test_f8523_close_recreate_cb; + ts->callback_arg = &s; + ts->events = CB_EVENT_CLOSED; + queue_init(&ts->sock.tcp.rxbuf, ts->rxmem, RXBUF_SIZE, ts->sock.tcp.ack); + + test_f8523_recreated_fd = -1; + + /* poll Step 3 dispatches the deferred CB_EVENT_CLOSED; the callback closes + * the socket and re-creates a fresh one in the same slot. */ + (void)wolfIP_poll(&s, 1); + + /* The callback ran and created a fresh socket. */ + ck_assert_int_ge(test_f8523_recreated_fd, 0); + /* The fresh socket must have survived the dispatcher's post-callback reap. */ + ck_assert_uint_eq(s.tcpsockets[SOCKET_UNMARK(test_f8523_recreated_fd)].proto, + (uint8_t)WI_IPPROTO_TCP); +} +END_TEST + START_TEST(test_tcp_last_ack_partial_ack_keeps_socket_and_timer) { struct wolfIP s; diff --git a/src/wolfip.c b/src/wolfip.c index 2e2230ba..c0f65164 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -11317,20 +11317,30 @@ static void handle_socket_callbacks(struct wolfIP *s) if ((ts->sock.tcp.state == TCP_CLOSED) && !(ts->events & CB_EVENT_CLOSED)) continue; { + tsocket_cb cb = ts->callback; + void *cb_arg = ts->callback_arg; uint16_t events = ts->events; ts->events = 0; - ts->callback(i | MARK_TCP_SOCKET, events, ts->callback_arg); - } - - /* Now that CB_EVENT_CLOSED has been delivered, reap the deferred-close - * socket. Disarm the callback first so close_socket() takes the plain - * teardown path instead of re-deferring (it re-arms close_notify_pending - * whenever a TCP socket still has a callback). A socket closed elsewhere - * is already memset (callback NULL) and never reaches this branch. */ - if (ts->sock.tcp.state == TCP_CLOSED) { - ts->callback = NULL; - ts->callback_arg = NULL; - close_socket(ts); + cb(i | MARK_TCP_SOCKET, events, cb_arg); + + /* Now that CB_EVENT_CLOSED has been delivered, reap the + * deferred-close socket - but only if the slot still holds the + * socket that was just dispatched. The callback may have closed + * this socket (freeing the slot) and allocated a fresh one in its + * place; a fresh socket is TCP_CLOSED by design, so reaping on + * state alone would destroy it. A replaced slot carries a + * different (or no) callback, so key the reap on the identity of + * the callback pair. Disarm the callback first so close_socket() + * takes the plain teardown path instead of re-deferring (it + * re-arms close_notify_pending whenever a TCP socket still has a + * callback). A socket closed elsewhere is already memset (callback + * NULL) and never reaches this branch. */ + if ((ts->callback == cb) && (ts->callback_arg == cb_arg) && + (ts->sock.tcp.state == TCP_CLOSED)) { + ts->callback = NULL; + ts->callback_arg = NULL; + close_socket(ts); + } } } From c977ef91a724b8e4d342fb6573e62a75f700d5eb Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 14:48:59 +0200 Subject: [PATCH 14/14] F-8525: reject oversized raw-socket payloads before narrowing len In the WOLFIP_RAWSOCKETS branch of wolfIP_sock_sendto(), total_len = ETH_HEADER_LEN + (uint32_t)len narrows the public size_t len to uint32_t before the total_len > LINK_MTU guard, but both payload copies - the ipheader_include memcpy and the non-ipheader memcpy(rip->data, buf, len) - use the original, un-narrowed len. On a 64-bit build a len of UINT32_MAX + 100 narrows to 100 for the MTU check (which passes) while the memcpy still attempts to copy the full, enormous len into the fixed-size frame buffer, overflowing it. Reject len > LINK_MTU as size_t before any narrowing conversion: a payload larger than the frame capacity cannot be sent anyway, and the bound is far below UINT32_MAX so the subsequent (uint32_t)len is exact and the total_len arithmetic cannot wrap. No behavioral change for any valid payload. The unit build enables WOLFIP_RAWSOCKETS unconditionally (unit_shared.c), so the test runs in all unit builds. test_raw_sendto_rejects_oversized_len _before_narrowing pins the bound: the oversized length is refused with -WOLFIP_EINVAL. Verified RED - with the bound check removed the same call segfaults (signal 11) in the payload memcpy - and GREEN. Plain, IP_MULTICAST and VLAN unit builds pass. --- src/test/unit/unit.c | 3 +++ src/test/unit/unit_tests_branches.c | 34 +++++++++++++++++++++++++++++ src/wolfip.c | 6 +++++ 3 files changed, 43 insertions(+) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index ac160a64..ada2bb51 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -1155,6 +1155,9 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_icmp_input_echo_reply_path_filter_at_eth); tcase_add_test(tc_core, test_ip_recv_with_options_oversize_dropped); tcase_add_test(tc_core, test_wolfip_recv_on_null_stack_returns); +#if WOLFIP_RAWSOCKETS + tcase_add_test(tc_core, test_raw_sendto_rejects_oversized_len_before_narrowing); +#endif /* Socket API arms: TCP, RAW, PACKET */ tcase_add_test(tc_core, test_register_callback_tcp_stores_handle); diff --git a/src/test/unit/unit_tests_branches.c b/src/test/unit/unit_tests_branches.c index 64f616cb..585dd701 100644 --- a/src/test/unit/unit_tests_branches.c +++ b/src/test/unit/unit_tests_branches.c @@ -2607,3 +2607,37 @@ START_TEST(test_wolfip_recv_on_null_stack_returns) wolfIP_recv_on(NULL, TEST_PRIMARY_IF, buf, sizeof(buf)); } END_TEST + +#if WOLFIP_RAWSOCKETS +/* F-8525: the raw-socket sendto path must reject a payload that cannot fit + * in a frame before narrowing len to uint32_t. A size_t len above the + * LINK_MTU-derived bound wraps in the total_len computation, slips past the + * LINK_MTU guard, and lets the payload memcpy overflow the fixed-size frame + * buffer. len = UINT32_MAX + 100 narrows to 100 (which would pass the MTU + * guard) but must be rejected before any memcpy. The overflow itself cannot + * be exercised in a unit test (it needs a >4GB buffer), so this pins the + * new size_t bound: the oversized length is refused, not narrowed. */ +START_TEST(test_raw_sendto_rejects_oversized_len_before_narrowing) +{ + struct wolfIP s; + int fd; + uint8_t buf[8]; + struct wolfIP_sockaddr_in sin; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + fd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_RAW, 0); + ck_assert_int_ge(fd, 0); + + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_addr.s_addr = ee32(0x0A000002U); + + ck_assert_int_eq(wolfIP_sock_sendto(&s, fd, buf, (size_t)UINT32_MAX + 100, 0, + (struct wolfIP_sockaddr *)&sin, sizeof(sin)), -WOLFIP_EINVAL); +} +END_TEST +#endif /* WOLFIP_RAWSOCKETS */ + diff --git a/src/wolfip.c b/src/wolfip.c index c0f65164..2a5988d0 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -6900,6 +6900,12 @@ int wolfIP_sock_sendto(struct wolfIP *s, int sockfd, const void *buf, size_t len return -WOLFIP_EINVAL; if (len == 0) return -WOLFIP_EINVAL; + /* Reject payloads that cannot fit in a frame before narrowing len to + * uint32_t below: a size_t len above the LINK_MTU-derived bound wraps + * in the total_len computation, slips past the LINK_MTU guard, and + * lets the payload memcpy overflow the fixed-size frame buffer. */ + if (len > (size_t)LINK_MTU) + return -WOLFIP_EINVAL; if (sin) { if (addrlen < sizeof(struct wolfIP_sockaddr_in)) return -WOLFIP_EINVAL;