More various fixes from the analyzers#105
Merged
danielinux merged 6 commits intowolfSSL:masterfrom Apr 28, 2026
Merged
Conversation
Contributor
gasbytes
commented
Apr 23, 2026
- Mask the reserved nibble on every receive-side read of tcp->hlen via a new tcp_data_offset_bytes helper so segments carrying non-zero reserved bits are processed identically to those without, per RFC 9293 section 3.1 MUST-ignore rule, with a new test_tcp_input_ignores_reserved_bits_in_hlen pinning the contract and four pre-existing option-parser tests re-aligned to legal 4-byte header encodings.
- Set the IP DF bit on locally-originated ICMP replies in wolfIP_send_ttl_exceeded, wolfIP_send_port_unreachable, and the icmp_input echo-reply path to match the tcp_send_reset_reply precedent, with three new *_sets_df tests pinning the contract.
- Add a bind_port_in_use collision scan across tcpsockets/udpsockets/icmpsockets in wolfIP_sock_bind so duplicate binds to an already-owned (local_ip, port) pair are rejected with -1 instead of silently succeeding, with test_sock_bind_tcp_port_collision_rejected pinning the contract.
- Enforce a 576-octet next-hop MTU floor in icmp_try_deliver_tcp_error so spoofed ICMP Fragmentation Needed messages cannot push peer_mss below the RFC 879 / RFC 9293 default of 536, with a new test_icmp_input_dest_unreach_frag_needed_below_floor_preserves_peer_mss covering the reject path.
- add test_ip_recv_drops_zero_source to cover the ipaddr_any && !dhcp_is_running branch of the ip_recv rfc 1122 3.2.1.3 source address validation
…s_running branch of the ip_recv rfc 1122 3.2.1.3 source address validation
…so spoofed ICMP Fragmentation Needed messages cannot push peer_mss below the RFC 879 / RFC 9293 default of 536, with a new test_icmp_input_dest_unreach_frag_needed_below_floor_preserves_peer_mss covering the reject path.
…mpsockets in wolfIP_sock_bind so duplicate binds to an already-owned (local_ip, port) pair are rejected with -1 instead of silently succeeding, with test_sock_bind_tcp_port_collision_rejected pinning the contract.
…tl_exceeded, wolfIP_send_port_unreachable, and the icmp_input echo-reply path to match the tcp_send_reset_reply precedent, with three new *_sets_df tests pinning the contract.
…a new tcp_data_offset_bytes helper so segments carrying non-zero reserved bits are processed identically to those without, per RFC 9293 §3.1's MUST-ignore rule, with a new test_tcp_input_ignores_reserved_bits_in_hlen pinning the contract and four pre-existing option-parser tests re-aligned to legal 4-byte header encodings.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR applies several analyzer-driven correctness and robustness fixes across the wolfIP IPv4/TCP/ICMP stack, and adds unit tests to pin the new behavior (TCP reserved-bit handling, ICMP DF behavior, bind collision rejection, ICMP PTB MTU floor, and zero-source validation).
Changes:
- Add
tcp_data_offset_bytes()and use it in multiple receive-side TCP header-length reads to ignore reserved bits per RFC 9293, plus update option-parsing tests and add a new reserved-bits test. - Set IPv4 DF on locally generated ICMP error/reply packets in several paths and add tests asserting DF is set.
- Add a bind-time
(local_ip, port)collision check (within each socket table) so duplicate binds are rejected, plus add a regression test. - Enforce a 576-byte next-hop MTU floor for ICMP “Fragmentation Needed” to prevent reducing TCP peer MSS below the IPv4 default, plus add a regression test.
- Add a unit test for dropping zero-source IPv4 packets.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/wolfip.c | Core stack changes: TCP data-offset masking helper, ICMP DF flagging, bind collision detection, and ICMP PTB MTU-floor enforcement. |
| src/test/unit/unit_tests_tcp_flow.c | Adds reserved-bits-in-hlen test and realigns multiple TCP option-parser tests to valid 4-byte header encodings. |
| src/test/unit/unit_tests_tcp_ack.c | Adds DF-flag assertion test for TTL-exceeded ICMP generation. |
| src/test/unit/unit_tests_proto.c | Adds DF-flag assertion test for port-unreachable ICMP generation. |
| src/test/unit/unit_tests_dns_dhcp.c | Adds DF-flag assertion test for echo reply path and adds MTU-floor regression test for ICMP frag-needed handling. |
| src/test/unit/unit_tests_api.c | Adds bind collision regression test and adds zero-source drop regression test for ip_recv. |
| src/test/unit/unit.c | Registers the newly added unit tests in the suite. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- correct MTU comment
3f91064 to
571a80e
Compare
danielinux
approved these changes
Apr 28, 2026
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.