Repository navigation
Fix five memory-safety issues from Forescout (Vedere Labs) disclosure (FSCT-2026-0017/0018/0019/0020/0022) - #81
Merged
danielinux merged 6 commits intoSep 8, 2026
Conversation
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)
… 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)
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)
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)
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)
Member
|
Thanks a lot @fralasp for providing the patch upon public disclosure. The fixes look good and it seems to fix the claim you reported. One minor change requested for our CI gate: The The formatter strips the blank line between the while ((ptr < end) && (*ptr != 0))
ptr++;
-
/* No terminator within the response: reject instead of walking past it. */
if (ptr >= end)
return NULL;Dropping that blank line makes the whole tree pass with the exact CI command |
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.
Contributor
Author
|
@danielinux the issue should be fixed now |
danielinux
added a commit
that referenced
this pull request
Sep 9, 2026
These five memory-safety fixes shipped in PR #81; list them in the 2.2 fixed section alongside the three DNS/mDNS CVEs.
danielinux
added a commit
that referenced
this pull request
Sep 11, 2026
These five memory-safety fixes shipped in PR #81; list them in the 2.2 fixed section alongside the three DNS/mDNS CVEs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes five issues reported by Forescout Research (Vedere Labs), disclosure FSCT-2026-0017/0018/0019/0020/0022. Five independent commits, one per issue, touching disjoint files.
FSCT-2026-0017 — Heap overflow in IP fragment reassembly (
modules/pico_fragments.c)pico_fragments_reassemble()allocates(uint16_t)(header_length + len), truncating the total mod 65536, while the copy loop writes the full length; the cumulative sum is never bounded. A crafted contiguous fragment chain exceeding 65535 bytes overflows the undersized heap buffer with attacker-controlled payload. Remote, pre-auth, withPICO_SUPPORT_IPV4FRAG/IPV6FRAG. Fix: reject totals that do not fit a 16-bit frame length, written aslen > (65535 - header_length)so the test cannot itself wrap; the fragment tree is left to the existing expiration timer, as on the allocation-failure path below it.FSCT-2026-0019 — Unsigned underflow in
pico_udp_recv()→ OOB read (modules/pico_udp.c)The payload length is
(uint16_t)(transport_len - 8)with no check thattransport_len >= 8. A datagram claiming 0..7 transport bytes (checksum 0, valid per RFC 768) underflows to ~65528, driving amemcpythat reads far past the frame. Remote, pre-auth, default build. Fix: treat a transport length below the UDP header as a zero-length payload.FSCT-2026-0022 — Off-by-
option_lenbound inpico_ipv4_process_in()→ OOB read (modules/pico_ipv4.c)max_allowedomitsoption_len, but the transport area startsoption_lenbytes further in, so the check admits a transport region up to 40 bytes past the frame; a downstream reader (e.g. the ICMP checksum) then reads out of bounds. Remote, pre-auth, single IPv4 packet with options. Fix: subtractoption_lenfrom the bound — the precedingnet_lenfit-check guarantees this cannot underflow.FSCT-2026-0020 — Unbounded name walk in
pico_dns_client_seek()→ OOB read (modules/pico_dns_client.c)The seek walks to a NUL terminator with no bound, on the raw response before header/ID validation, so a response with no terminator reads past the static
dns_responsebuffer. Low practical severity (usually stops on zero-initialized.bss); reported for hardening. Fix: pass the end of the buffer explicitly and return NULL if no terminator is found — both call sites already handle NULL.FSCT-2026-0018 —
pico_socket_accept()address-buffer overflow (test/examples/tcpecho.c,include/pico_socket.h)pico_socket_accept()sizes the peer-address copy by the socket family (16 bytes for IPv6) with no destination-size parameter, and the shippedtcpecho.cexample passes a 4-bytestruct pico_ip4buffer regardless of family, a 12-byte overflow on an IPv6 build. Fix (non-breaking): useunion pico_addressin the example, astcpbench.calready does, and document the buffer-size contract on the public declaration. A POSIX-styleaddrlenparameter would let the library bound the copy itself.