CHERIoT-Platform / CHERIoT-Platform/network-stack
packet_filter_ipv4 in firewall.cc allows a packet with an invalid IHL to bypass the firewall filter.
- 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:
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