More various fixes (F-*)#125
Merged
Merged
Conversation
…hakes per poll cycle) after the dos guard flag (under_load) - change the poll to check if under_load to include 8 max peers (from > to >=) this makes sure that when the peers are exactly 8, we still turn on the under_load flag, which means that it will require the peer to calculate the mac2 to prevent dos attacks and prevents the increase of the handshakes per_cycle before mac2 is calculated.
requests now only relipes will populate the cache and update the affected arp unit tests to match
… was up until 120s before refresh) initializing it with (UINT64_MAX - (uint64_t)WG_COOKIE_SECRET_MAX_AGE * 1000UL - 1)
…in a header value can no longer inject headers or smuggle body
…fore honouring an icmp port/prot_unreach
… an absent or mismatched server identifier can no longer deconfigure the lease this pretty much mirrors the guard already in dhcp_parse_ack
gasbytes
force-pushed
the
2026-05-25-findings-fixes
branch
from
c33f76c to
862ee3b
Compare
danielinux
approved these changes
Jun 3, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR delivers a set of security/correctness hardening fixes across wolfIP’s DHCP/TCP/ARP/HTTP paths and wolfGuard’s packet/cookie handling, along with accompanying unit test updates to lock in the new behaviors.
Changes:
- Tighten validation in DHCP NAK handling (server-id), ICMP error handling (TCP embedded SEQ window), ARP neighbor learning (reply-only), and HTTP percent-decoding (request-target only).
- Harden wolfGuard cookie secret initialization and load accounting; add IPv4 version check before dispatch.
- Extend unit tests to cover the new rejection/acceptance criteria and security regressions.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/wolfip.c | Adds null guards, tightens ICMP/TCP error acceptance, hardens DHCP NAK handling, and changes ARP learning behavior. |
| src/wolfguard/wolfguard.c | Adds IPv4 version validation for wg0 TX dispatch; adjusts under-load threshold behavior. |
| src/wolfguard/wg_packet.c | Moves handshake-per-cycle accounting to occur after under-load cookie enforcement. |
| src/wolfguard/wg_cookie.c | Initializes cookie secret birthdate to prevent MAC2 validation from a never-generated secret. |
| src/http/httpd.c | Limits percent-decoding to the request target and rejects CR/LF in the decoded path to prevent header/body injection. |
| src/test/unit/unit.c | Registers new unit tests and updates test count annotation. |
| src/test/unit/unit_wolfguard.c | Adds tests for cookie-zero-secret MAC2 bypass prevention, response-path load threshold, and non-IPv4 output rejection. |
| src/test/unit/unit_tests_tcp_state.c | Adds coverage for ICMP PORT_UNREACH embedded TCP SEQ validation (in-window vs out-of-window). |
| src/test/unit/unit_tests_tcp_ack.c | Updates ARP request neighbor-cache expectations (requests must not populate cache). |
| src/test/unit/unit_tests_proto.c | Updates ARP request handling expectations to match reply-only caching. |
| src/test/unit/unit_tests_ip_arp_recv.c | Reworks ARP request/reply caching tests and adds cache-poisoning regression coverage. |
| src/test/unit/unit_tests_dhcp_edges.c | Adds DHCP msg-type server-id validation tests for NAK handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
7812
to
+7816
| struct dhcp_msg req; | ||
| struct dhcp_option *opt = (struct dhcp_option *)(req.options); | ||
| struct wolfIP_sockaddr_in sin; | ||
| struct ipconf *primary = wolfIP_primary_ipconf(s); | ||
| uint64_t retry_at = s ? (s->last_tick + 1U) : 0; | ||
| int renewing = (s->dhcp_state == DHCP_RENEWING); | ||
| int rebinding = (s->dhcp_state == DHCP_REBINDING); | ||
| uint64_t retry_at = 0; |
Comment on lines
242
to
+246
| if (len < 20) | ||
| return -1; /* Too short for IPv4 header */ | ||
|
|
||
| if ((packet[0] >> 4) != 4) | ||
| return -1; |
Comment on lines
+773
to
+774
| /* Default-initialized checker: secret all-zero, birthdate 0 */ | ||
| wg_cookie_checker_init(&dev.cookie_checker, dev.static_public); |
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.
c33f76c - validate the dhcp server-id in dhcp_msg_type so a forged dhcpnak with an absent or mismatched server identifier can no longer deconfigure the lease this pretty much mirrors the guard already in dhcp_parse_ack
3ae5ec9 - validate the tcp sequence number against the half-open send window before honouring an icmp port/prot_unreach
b197590 scope http percent-decoding to the request target so an encoded CRLF in a header value can no longer inject headers or smuggle body
2dcf41a - initialize the secret_birthdate so that the mac2 is not forgeable (it was up until 120s before refresh) initializing it with (UINT64_MAX - (uint64_t)WG_COOKIE_SECRET_MAX_AGE * 1000UL - 1)
349d1f2 - add missing ipv4 correcteness check (nibble must be 4)
8710a1d - add missing null guard in dhcp_send_request
7558e58 - add missing null check in wolfip_poll
4bdeea6 - add missing null check in wolfip_register_callback
7955e6d - prevent arp cache poisoning by no longer learning neighbors from arp requests now only relipes will populate the cache and update the affected arp unit tests to match
e31c232 - move the dev->handshakes_per_cycle increment (counts how many handshakes per poll cycle) after the dos guard flag (under_load) - change the poll to check if under_load to include 8 max peers (from > to >=)