Mitigate ARP poisoning by preventing overwrite#58
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens wolfIP’s ARP neighbor learning to mitigate ARP-table poisoning by preventing existing neighbor entries from being overwritten by unsolicited ARP traffic, while still allowing updates when an ARP reply corresponds to a recently-sent ARP request.
Changes:
- Add tracking for recently-issued ARP requests (pending requests with TTL) to distinguish solicited vs unsolicited ARP replies.
- Prevent ARP requests and unsolicited ARP replies from overwriting an existing neighbor cache entry.
- Add unit tests covering: no-overwrite on unsolicited reply, overwrite allowed when reply matches a pending request, and no-overwrite on ARP request.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/wolfip.c | Adds pending-request tracking and changes ARP neighbor update rules to block unsolicited overwrites. |
| src/test/unit/unit.c | Adds/adjusts ARP unit tests to validate the new overwrite-prevention behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
When receiving unsolicited ARP traffic, wolfIP accepts neighbors addresses. This prevents overwriting/poisoning ARP tables when an attacker is trying to impersonate an existing neighbor. For latency/performance optimizations, wolfIP is currently still populating the neighbor cache based on valid ARP requests from neighbors, if neighbors were not known.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/wolfip.c:5284
arp_pending_record()is called before the send filter check and before verifying the ARP request is actually transmitted. IfwolfIP_filter_notify_eth()blocks sending (orll->sendis NULL), a pending entry is still recorded and an attacker can then send a spoofed ARP reply within the TTL that will be treated as solicited and allowed to update/overwrite the neighbor entry. Move the pending record to only occur after the send filter passes (and ideally only when the request is actually sent).
arp.tip = ee32(tip);
arp_pending_record(s, if_idx, tip);
if (ll->send) {
if (wolfIP_filter_notify_eth(WOLFIP_FILT_SENDING, s, if_idx, &arp.eth,
sizeof(struct arp_packet)) != 0)
return;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
src/wolfip.c:5299
arp_pending_record()is called before confirming the ARP request will actually be sent. Ifll->sendis NULL orwolfIP_filter_notify_eth()blocks the frame, the stack will still treat a later ARP reply as “pending” and allow a neighbor entry to be created/overwritten based on a request that never went out. Move the pending-recording to only occur after the filter allows the send (and only whenll->sendis non-NULL); ideally also avoid recording when the request is throttled/early-returned.
arp.tip = ee32(tip);
arp_pending_record(s, if_idx, tip);
if (ll->send) {
if (wolfIP_filter_notify_eth(WOLFIP_FILT_SENDING, s, if_idx, &arp.eth,
sizeof(struct arp_packet)) != 0)
return;
ll->send(ll, &arp, sizeof(struct arp_packet));
src/wolfip.c:5299
- The new “pending ARP request” tracking is security-sensitive, but the unit tests don’t currently cover the cases where an ARP request is not actually sent (e.g.
ll->send == NULLor the eth send filter drops it). Add tests to ensure no pending entry is recorded in those cases, so a forged ARP reply can’t be accepted as pending.
arp_pending_record(s, if_idx, tip);
if (ll->send) {
if (wolfIP_filter_notify_eth(WOLFIP_FILT_SENDING, s, if_idx, &arp.eth,
sizeof(struct arp_packet)) != 0)
return;
ll->send(ll, &arp, sizeof(struct arp_packet));
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
if the IP/MAC stay the same.
When receiving unsolicited ARP traffic, wolfIP
accepts neighbors addresses. This prevents overwriting/poisoning ARP tables when an attacker is trying to impersonate an existing neighbor.
For latency/performance optimizations, wolfIP is currently still populating the neighbor cache based on valid ARP requests from neighbors, if neighbors were not known.
Fenrir/226
edit: after Copilot's review, I decided to add an aging mechanism to limit issues with renewal of neighbors records. Also added tests to check aging.