- Problem
The function first reads the socket type and local port:
network_socket_kind(socket, kindPtr);
It then runs DNS:
int ret =
network_host_resolve(timeout, host->hostname, UseIPv6, addressPtr);
After DNS, it adds the firewall rule using the old port, but at this point, the socket might already been freed.
firewall_add_udpipv4_endpoint(
address.ipv4, kind.localPort, ntohs(host->port));
[network_socket_udp_authorise_host()](
|
SocketKind kind; |
|
CHERI::Capability kindPtr = &kind; |
|
kindPtr.permissions() &= {CHERI::Permission::Store}; |
|
// No need to check the return value here, potential errors will be |
|
// detected in the switch. |
|
network_socket_kind(socket, kindPtr); |
|
|
|
bool isIPv6 = false; |
|
switch (kind.protocol) |
|
{ |
|
default: |
|
case SocketKind::Invalid: |
|
case SocketKind::TCPIPv4: |
|
case SocketKind::TCPIPv6: |
|
return address; |
|
case SocketKind::UDPIPv4: |
|
break; |
|
case SocketKind::UDPIPv6: |
|
isIPv6 = true; |
|
break; |
|
} |
|
|
|
CHERI::Capability addressPtr = &address; |
|
addressPtr.permissions() &= {CHERI::Permission::Store}; |
|
firewall_permit_dns(); |
|
int ret = |
|
network_host_resolve(timeout, host->hostname, UseIPv6, addressPtr); |
|
firewall_permit_dns(false); |
|
if ((ret < 0) || (address.kind == NetworkAddress::AddressKindInvalid)) |
|
{ |
|
Debug::log("Failed to resolve host"); |
|
return address; |
|
} |
|
if (isIPv6 != (address.kind == NetworkAddress::AddressKindIPv6)) |
|
{ |
|
Debug::log("Host address does not match socket type"); |
|
return address; |
|
} |
|
|
|
if (isIPv6) |
|
{ |
|
if constexpr (!UseIPv6) |
|
{ |
|
Debug::log("IPv6 is not supported"); |
|
return {NetworkAddress::AddressKindInvalid}; |
|
} |
|
else |
|
{ |
|
firewall_add_udpipv6_endpoint( |
|
address.ipv6, kind.localPort, ntohs(host->port)); |
|
} |
|
} |
|
else |
|
{ |
|
Debug::log("Adding address {}.{}.{}.{} to firewall", |
|
address.ipv4 & 0xFF, |
|
(address.ipv4 >> 8) & 0xFF, |
|
(address.ipv4 >> 16) & 0xFF, |
|
(address.ipv4 >> 24) & 0xFF); |
|
firewall_add_udpipv4_endpoint( |
|
address.ipv4, kind.localPort, ntohs(host->port)); |
)
-
Why is it bad?
Another thread can close the socket while DNS is running. Close removes the old endpoints, but this function can add a new endpoint after close has finished. If the port is reused, the rule may apply to the wrong socket.
-
Suggested fix
- DNS resolve;
- locks the socket;
- checks that it is still open and has the same port;
- adds the firewall endpoint;
- unlocks the socket.
The function first reads the socket type and local port:
network_socket_kind(socket, kindPtr);It then runs DNS:
After DNS, it adds the firewall rule using the old port, but at this point, the socket might already been freed.
firewall_add_udpipv4_endpoint( address.ipv4, kind.localPort, ntohs(host->port));[network_socket_udp_authorise_host()](
network-stack/lib/netapi/NetAPI.cc
Lines 220 to 280 in 58425e6
Why is it bad?
Another thread can close the socket while DNS is running. Close removes the old endpoints, but this function can add a new endpoint after close has finished. If the port is reused, the rule may apply to the wrong socket.
Suggested fix