CHERIoT-Platform / CHERIoT-Platform/network-stack

SmallTable::insert() keeps a dangling capability to the freed buffer after resizing

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

Description

### Summary
`SmallTable::insert()` does not update the table’s `buffer` member after resizing. When the ninth element is inserted, the table keeps a dangling capability to the freed allocation, which can crash the Firewall.

### Root cause
[`resize_if_needed()` returns a new allocation (and does not update the member `buffer`), but `SmallTable::insert()` only stores it in the local variable `currentBase`](https://github.com/CHERIoT-Platform/network-stack/blob/588106a83e05b70ebc9a845c5d6aec4fc303a4b4/lib/firewall/firewall.cc#L242). So the returned capability is never assigned back to the `buffer` member.

### Affected component
`lib/firewall/firewall.cc`, the internal `SmallTable` implementation is used by the firewall endpoint tables (e.g. `firewall_add_udpipv4_endpoint` uses the endpoint table).

### Impact
`SmallTable` initially has a capacity of 8. The ninth insertion triggers a resize: the old allocation is freed and a larger array is allocated, but `buffer` is left pointing to the freed allocation. The old allocation is marked in the revocation bitmap, and the stale capability’s tag becomes 0.

Functions such as `firewall_add_udpipv4_endpoint()` may grow the endpoint table. Later access to the table through the stale `buffer` capability can trigger a hardware exception and kill this compartment.

### PoC

```cpp
void test_small_table_growth()
{
Debug::log("Testing SmallTable growth");

SmallTable table;

for (int i = 0; i < 8; ++i)
{
table.insert(i);
}

Debug::log(
"Before ninth insertion: size {}, capacity {}",
table.size(),
table.capacity());

table.insert(8);

Debug::log(
"After ninth insertion: size {}, capacity {}",
table.size(),
table.capacity());

Debug::log("About to dereference dangling table");
int first = *table.begin();
Debug::log("Dereference survived, value {}", first);
}
```

Output:

Image

The ninth insertion leaves the table with size 9 and capacity 8 (capacity > size). And later access triggers the fault.
### Suggested fix

Copy the returned capability back to the table member before updating the size:

```cpp
T *currentBase = static_cast(
resize_if_needed(base(), currentSize, capacity(), sizeof(T)));

buffer = currentBase;

SmallTableBase::insert(
currentBase,
currentSize * sizeof(T),
&element,
sizeof(T));

set_size(currentSize + 1);
```

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in lib/firewall/firewall.cc at the SmallTable implementation and the resize_if_needed() call in SmallTable::insert(). Reproduce the issue with the test_small_table_growth() PoC, focusing on the ninth insertion and the subsequent table dereference. Done means the table retains the resized allocation, reports consistent size and capacity, and the later access does not fault.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.