From d25c4c2ff836c967d9b1f7527be052954ba29446 Mon Sep 17 00:00:00 2001 From: Francesco La Spina Date: Tue, 8 Sep 2026 12:02:00 +0200 Subject: [PATCH 1/6] Fix heap buffer overflow in IP fragment reassembly (uint16_t truncation) pico_fragments_reassemble() computes the frame allocation size as (uint16_t)(header_length + len), truncating the cumulative datagram length mod 65536, while the copy loop that follows writes the full, un-truncated length. pico_fragments_check_complete() accumulates the total with only a contiguity check and no upper bound, so a crafted chain of contiguous fragments whose reassembled size exceeds 65535 (reachable because the RFC offset+length <= 65535 limit is never enforced) allocates an undersized buffer and overflows the heap with attacker-controlled fragment payloads. Reject datagrams whose reassembled size does not fit a 16-bit frame length before the truncating allocation. The test is written as len > (65535 - header_length) so it cannot itself wrap. The fragment tree is left untouched and reclaimed by the existing expiration timer, matching the allocation-failure path below; emptying it here would leave that timer pending on an empty tree, and it would later fire on an unrelated reassembly. A strictly RFC-legal 65535-byte datagram still reassembles. Reachable remotely, pre-authentication, with PICO_SUPPORT_IPV4FRAG / PICO_SUPPORT_IPV6FRAG enabled. Reported by Forescout Research (Vedere Labs) - FSCT-2026-0017. (cherry picked from commit 850d451d0b867a1898b5d9824d88be9aeabf275b) --- modules/pico_fragments.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/modules/pico_fragments.c b/modules/pico_fragments.c index 704ffa7e..be4f4cd0 100644 --- a/modules/pico_fragments.c +++ b/modules/pico_fragments.c @@ -329,6 +329,14 @@ static int pico_fragments_reassemble(struct pico_tree *tree, unsigned int len, u return -3; } + /* A reassembled datagram must fit the 16-bit frame length. Without this + * check the (uint16_t) cast below truncates the allocation while the copy + * loop writes the full length, overflowing the heap buffer. The fragments + * are left for the expiration timer, as on allocation failure below. */ + if (len > (65535U - header_length)) { + return -4; + } + full = pico_frame_alloc((uint16_t)(header_length + len)); if (full) { full->net_hdr = full->buffer; From 540507bb4de67c1e07646f840a8f6eac29e62171 Mon Sep 17 00:00:00 2001 From: Francesco La Spina Date: Tue, 8 Sep 2026 12:15:16 +0200 Subject: [PATCH 2/6] Fix address-buffer overflow in pico_socket_accept() example; document contract pico_socket_accept() copies the peer address into the caller-supplied orig buffer, sizing the copy by the socket's address family (16 bytes for an IPv6 socket) with no destination-size parameter. The shipped example test/examples/tcpecho.c passes a 4-byte struct pico_ip4 buffer regardless of family, so an IPv6 build overflows it by 12 bytes with the attacker-controlled peer address when a remote peer connects. Use a family-agnostic union pico_address buffer in the example (matching the correct pattern already used in tcpbench.c), and document on the public pico_socket_accept() declaration that orig must be large enough for the socket family's address. A destination-size (POSIX addrlen) parameter would let the library bound the copy itself, but that is a breaking API change; this commit fixes the shipped vulnerable caller and documents the contract without changing the ABI. Reported by Forescout Research (Vedere Labs) - FSCT-2026-0018. (cherry picked from commit 4216993e49bfc756c2057c1311c02cbf5e408566) --- include/pico_socket.h | 4 ++++ test/examples/tcpecho.c | 8 ++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/include/pico_socket.h b/include/pico_socket.h index 1bd7a1f8..d167bbf5 100644 --- a/include/pico_socket.h +++ b/include/pico_socket.h @@ -234,6 +234,10 @@ int pico_socket_getpeername(struct pico_socket *s, void *remote_addr, uint16_t * int pico_socket_connect(struct pico_socket *s, const void *remote_addr, uint16_t remote_port); int pico_socket_listen(struct pico_socket *s, const int backlog); +/* `orig` must point to a buffer large enough for the peer address of the + * socket's address family: sizeof(struct pico_ip4) for IPv4, sizeof(struct + * pico_ip6) for IPv6. Use `union pico_address` to be safe for either family; + * a smaller buffer is overflown by the family-sized copy (FSCT-2026-0018). */ struct pico_socket *pico_socket_accept(struct pico_socket *s, void *orig, uint16_t *port); int8_t pico_socket_del(struct pico_socket *s); diff --git a/test/examples/tcpecho.c b/test/examples/tcpecho.c index 7a71c7c3..745054ce 100644 --- a/test/examples/tcpecho.c +++ b/test/examples/tcpecho.c @@ -58,7 +58,11 @@ void cb_tcpecho(uint16_t ev, struct pico_socket *s) struct pico_socket *sock_a = { 0 }; - struct pico_ip4 orig = { + /* Family-agnostic buffer: pico_socket_accept() copies the peer address + * sized by the socket's family (16 bytes for IPv6), so a bare + * struct pico_ip4 here would be overflowed by an IPv6 peer + * (FSCT-2026-0018). */ + union pico_address orig = { 0 }; uint16_t port = 0; @@ -68,7 +72,7 @@ void cb_tcpecho(uint16_t ev, struct pico_socket *s) int yes = 1; sock_a = pico_socket_accept(s, &orig, &port); - pico_ipv4_to_string(peer, orig.addr); + pico_ipv4_to_string(peer, orig.ip4.addr); printf("Connection established with %s:%d.\n", peer, short_be(port)); pico_socket_setoption(sock_a, PICO_TCP_NODELAY, &yes); /* Set keepalive options */ From d04bf0896febcfb880abf2bc631296959caf680a Mon Sep 17 00:00:00 2001 From: Francesco La Spina Date: Tue, 8 Sep 2026 12:04:18 +0200 Subject: [PATCH 3/6] Fix unsigned integer underflow in pico_udp_recv() (out-of-bounds read) pico_udp_recv() computes the payload length as (uint16_t)(f->transport_len - sizeof(struct pico_udp_hdr)) with no check that transport_len is at least the 8-byte UDP header. The IPv4 input path stores transport_len from the network hdr->len field with only an upper-bound sanity check, so a crafted IPv4/UDP datagram whose IP total-length claims 0..7 transport bytes (the real 8-byte header is present so the port demux succeeds; the UDP checksum is set to 0, valid per RFC 768) makes the subtraction underflow to ~65528. That bogus length then drives the memcpy that fills the application buffer, reading far past the received frame into adjacent memory. Treat a transport length shorter than the UDP header as a zero-length payload, so recv returns no data and the malformed frame is discarded. Reachable remotely, pre-authentication, default build (PICO_SUPPORT_UDP + PICO_SUPPORT_IPV4). Reported by Forescout Research (Vedere Labs) - FSCT-2026-0019. (cherry picked from commit 0f4ef19e0e61471a8b8549f73e0bc8a008696f2e) --- modules/pico_udp.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/modules/pico_udp.c b/modules/pico_udp.c index f00a421e..006de3c6 100644 --- a/modules/pico_udp.c +++ b/modules/pico_udp.c @@ -199,7 +199,15 @@ uint16_t pico_udp_recv(struct pico_socket *s, void *buf, uint16_t len, void *src if (f) { if (!f->payload_len) { f->payload = f->transport_hdr + sizeof(struct pico_udp_hdr); - f->payload_len = (uint16_t)(f->transport_len - sizeof(struct pico_udp_hdr)); + /* A datagram whose transport length is smaller than the UDP header + * carries no payload; without this guard the subtraction underflows + * to ~65528 and the copy below reads far past the frame + * (FSCT-2026-0019). */ + if (f->transport_len < sizeof(struct pico_udp_hdr)) { + f->payload_len = 0; + } else { + f->payload_len = (uint16_t)(f->transport_len - sizeof(struct pico_udp_hdr)); + } } udp_dbg("expected: %d, got: %d\n", len, f->payload_len); From 3c0e7d5c49a40e382856e41b7a833c7e51509e86 Mon Sep 17 00:00:00 2001 From: Francesco La Spina Date: Tue, 8 Sep 2026 12:10:47 +0200 Subject: [PATCH 4/6] Fix unbounded name walk in pico_dns_client_seek() (out-of-bounds read) pico_dns_client_seek() advanced a pointer until it found a NUL byte with no bound on how far it could walk, and it runs on the raw network-received response (dns_response) before the DNS header/ID are validated. A crafted response with no NUL byte from the name field to the end of the buffer made the loop read past the end of the static dns_response buffer. Pass the end of the response buffer explicitly and stop there, returning NULL when no terminator is found within it. Both call sites already handle a NULL return: pico_dns_client_check_qsuffix() rejects a NULL suffix, and pico_dns_client_seek_suffix() breaks out of its loop and returns NULL. Reachable by an attacker able to answer the client's outstanding query (rogue/on-path DNS server); requires PICO_SUPPORT_DNS_CLIENT. Reported by Forescout Research (Vedere Labs) - FSCT-2026-0020. (cherry picked from commit c424c1f7de67586775397e04578a27a2212ce697) --- modules/pico_dns_client.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/modules/pico_dns_client.c b/modules/pico_dns_client.c index 5e518ade..47c0c9bd 100644 --- a/modules/pico_dns_client.c +++ b/modules/pico_dns_client.c @@ -237,14 +237,19 @@ static struct pico_dns_query *pico_dns_client_find_query(struct pico_stack *S, u return NULL; } -/* seek end of string */ -static char *pico_dns_client_seek(char *ptr) +/* seek end of string, without reading past the end of the response buffer */ +static char *pico_dns_client_seek(char *ptr, const char *end) { - if (!ptr) + if (!ptr || !end) return NULL; - while (*ptr != 0) + while ((ptr < end) && (*ptr != 0)) ptr++; + + /* No terminator within the response: reject instead of walking past it. */ + if (ptr >= end) + return NULL; + return ptr + 1; } @@ -358,7 +363,7 @@ static char *pico_dns_client_seek_suffix(char *suf, struct pico_dns_header *pre, case PICO_DNS_LABEL: dns_dbg("DNS: label\n"); - suf = pico_dns_client_seek(suf); + suf = pico_dns_client_seek(suf, (const char *)pre + PICO_IP_MRU); break; default: @@ -602,7 +607,7 @@ static void pico_dns_client_callback(uint16_t ev, struct pico_socket *s) /* FIX: what if the query is not a PTR query? */ domain = (char *)header + sizeof(struct pico_dns_header); - qsuffix = (struct pico_dns_question_suffix *)pico_dns_client_seek(domain); + qsuffix = (struct pico_dns_question_suffix *)pico_dns_client_seek(domain, dns_response + sizeof(dns_response)); /* valid asuffix is determined dynamically later on */ if (pico_dns_client_check_qsuffix(qsuffix, q) < 0) From 23c7fd44123f7e6c169b99504ee858471495c03b Mon Sep 17 00:00:00 2001 From: Francesco La Spina Date: Tue, 8 Sep 2026 12:06:47 +0200 Subject: [PATCH 5/6] Fix off-by-option_len bounds check in pico_ipv4_process_in() (OOB read) pico_ipv4_process_in() advances the transport header past the IP options (net_hdr + PICO_SIZE_IP4HDR + option_len) but computes its length bound max_allowed as (buffer_len - net_hdr_offset - PICO_SIZE_IP4HDR), without subtracting option_len. The space actually available for the transport area is max_allowed - option_len, so the check transport_len > max_allowed admits a transport region extending up to option_len (<= 40) bytes past the end of the frame. A downstream handler that reads transport_len bytes from transport_hdr - e.g. pico_icmp4_checksum() - then reads out of bounds. Subtract option_len from the bound. The preceding net_len fit-check guarantees max_allowed >= option_len, so the subtraction cannot underflow. Reachable remotely, pre-authentication, single crafted IPv4 packet with IP options, default build. Reported by Forescout Research (Vedere Labs) - FSCT-2026-0022. (cherry picked from commit 7bb106ef19adf35c75993b2524e652b79264da31) --- modules/pico_ipv4.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/pico_ipv4.c b/modules/pico_ipv4.c index aa46871b..d8f3683a 100644 --- a/modules/pico_ipv4.c +++ b/modules/pico_ipv4.c @@ -675,7 +675,11 @@ static int pico_ipv4_process_in(struct pico_stack *S, struct pico_protocol *self f->frag = short_be(hdr->frag); #endif - if (f->transport_len > max_allowed) { + /* max_allowed measures the space after the fixed IP header, but the transport + * area starts option_len bytes further in; subtract the options so the check + * cannot admit a transport region running past the frame (FSCT-2026-0022). + * The net_len fit-check above guarantees max_allowed >= option_len. */ + if (f->transport_len > (uint16_t)(max_allowed - option_len)) { pico_frame_discard(f); return 0; /* Packet is discarded due to unfeasible length */ } From d366ce2727df9cfc9e130e814526b47653d12ba3 Mon Sep 17 00:00:00 2001 From: Francesco La Spina Date: Tue, 8 Sep 2026 17:34:07 +0200 Subject: [PATCH 6/6] Satisfy uncrustify check in pico_dns_client_seek() The uncrustify CI gate (0.78.1) removes the blank line between the search loop and the comment that belongs to the following bounds check, so the tree failed the check with FAIL: ./modules/pico_dns_client.c (File size changed from 24557 to 24556) Drop that blank line. No functional change. --- modules/pico_dns_client.c | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/pico_dns_client.c b/modules/pico_dns_client.c index 47c0c9bd..6a7600b5 100644 --- a/modules/pico_dns_client.c +++ b/modules/pico_dns_client.c @@ -245,7 +245,6 @@ static char *pico_dns_client_seek(char *ptr, const char *end) while ((ptr < end) && (*ptr != 0)) ptr++; - /* No terminator within the response: reject instead of walking past it. */ if (ptr >= end) return NULL;