CHERIoT-Platform / CHERIoT-Platform/network-stack

packet_filter_ipv4 in firewall.cc allows a packet with an invalid IHL to bypass the firewall filter.

Open Beginner friendly
#96 1 comment 2 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
9
Forks
14
Avg merge
6d 17h
Merged PRs (30d)
1

Description

### Summary
The `packet_filter_ipv4()` filter is vulnerable in the IP header length sanity check, it is not exploitable, but better to fix it to avoid the "Swiss Cheese".

### Root cause
[`packet_filter_ipv4()` uses `sizeof(ipv4Header)` where `ipv4Header` is a pointer](https://github.com/CHERIoT-Platform/network-stack/blob/588106a83e05b70ebc9a845c5d6aec4fc303a4b4/lib/firewall/firewall.cc#L671)

### Affected component
`lib/firewall/firewall.cc`, IPv4 TCP/UDP filtering path in `packet_filter_ipv4()`.

### Impact
Malformed IPv4 packets with invalid IHL may pass the header-length check. Any client can build a malformed packet and make the firewall parse fake TCP/UDP fields from inside the IPv4 header to pass this filter. But later [checks](https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/e512b5cc306b491893743ae2e151ccdf76f9b475/source/FreeRTOS_IP.c#L1864) will intercept the bad-IHL packet, so this bug is not exploitable.

### PoC
I added a local test that calls the `packet_filter_ipv4()` with a crafted invalid IPv4 packet:
```cpp
myTest.cc
void __cheri_compartment("bad_ihl_test") run_test()
{
Debug::log("Calling firewall bad-IHL test");

bool triggered = firewall_test_bad_ihl_filter();

if (triggered)
{
Debug::log("FINAL: BUG TRIGGERED");
}
else
{
Debug::log("FINAL: bug not triggered");
}

while (true) {}
}
```
```cpp
firewall.cc (temporary test)
bool __cheri_compartment("Firewall") firewall_test_bad_ihl_filter()
{
Debug::log("Running packet_filter_ipv4 test");

EndpointsTable::instance().clear(IPProtocolNumber::TCP);
currentClientCount = 0;

uint16_t serverPort = htons(80);
EndpointsTable::instance().add_server_port(serverPort);

uint8_t p[64] = {};

/**
* Invalid IPv4 header:
* Version = 4
* IHL = 4
* offset = 4 * 4 = 16
*
* Valid IPv4 header must be at least 20 bytes. (IHL >= 5)
*/
p[0] = 0x44; // Version = 4 ; IHL = 4

// Total length = 64.
p[2] = 0x00;
p[3] = 64;

p[8] = 64; // TTL
p[9] = static_cast(IPProtocolNumber::TCP); // protocol = TCP

// Source IP.
p[12] = 8;
p[13] = 8;
p[14] = 8;
p[15] = 8;

/**
* Since offset=16, buggy filter reads fake TCP ports here.
*
* fake source port = 0x0035 = 48 + 5 = 53
* fake dest port = 0x0050 = 80
*/
p[16] = 0x00;
p[17] = 0x35;
p[18] = 0x00;
p[19] = 0x50;

/**
* fake TCP flags at offset + 12 = 28.
* 0x5002 = SYN, no ACK.
*/
p[28] = 0x50;
p[29] = 0x02;

auto *ip = reinterpret_cast(p);

Debug::log("bad packet: body_offset={}, IPv4Header={}, pointerSize={}",
ip->body_offset(),
sizeof(IPv4Header),
sizeof(ip));

auto result = packet_filter_ipv4(
p,
sizeof(p),
&IPv4Header::sourceAddress,
&TCPUDPCommonPrefix::destinationPort,
&TCPUDPCommonPrefix::sourcePort,
false);

Debug::log("packet_filter_ipv4 returned {}", static_cast(result));

if ((result & ForwardFlags::ForwardNetworkStack) != 0)
{
Debug::log("BUG TRIGGERED: packet_filter_ipv4 accepted bad IHL packet");
return true;
}

Debug::log("Bad IHL packet was dropped");
return false;
}
```
Here is the output:

Image

On the buggy code, the filter accepts the invalid packet.

### Suggested fix

Replace:
```cpp
if (ipv4Header->body_offset() < sizeof(ipv4Header))
```
with
```cpp
if (ipv4Header->body_offset() < sizeof(*ipv4Header))
```

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in lib/firewall/firewall.cc at packet_filter_ipv4(), where the IPv4 header-length sanity check is shown. Review the temporary bad-IHL test in myTest.cc and the crafted packet described in the issue, then run the relevant firewall test or reproduce that case. Done means invalid-IHL packets fail the filter without changing valid IPv4 TCP/UDP filtering.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
networking, security
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.