From 88376e19928457338302667594a085838c55ccf3 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 12:58:40 +0200 Subject: [PATCH 1/7] 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 18feef22386b9ab0648a861097c2efd5e84787bb Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 13:01:55 +0200 Subject: [PATCH 2/7] 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 0f546a2c3a1e10fe3f46768062fa853dd7cefe15 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 13:09:33 +0200 Subject: [PATCH 3/7] 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 2a08f1830dba7abb18928167ea243178983df3e4 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 13:10:16 +0200 Subject: [PATCH 4/7] 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 9c8cbb8ef5be23186926ade510de7cd3e0632fa0 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 13:19:48 +0200 Subject: [PATCH 5/7] 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 235062d67dc3f78063b513dd81969f621a01717a Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 13:37:23 +0200 Subject: [PATCH 6/7] 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 cd34afa916c5ce47492fd75ca6a0c44b11df96ed Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 13:52:56 +0200 Subject: [PATCH 7/7] 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)) {