zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.57k stars 6.47k forks source link

net: dns: Out of bounds array access in DNS dispatcher #79042

Open mrodgers-witekio opened 1 day ago

mrodgers-witekio commented 1 day ago

Describe the bug

Depending on network configuration, it is possible to create an out of bounds array access in dns_dispatcher_register.

Target platforms:

To Reproduce

I encountered the issue with a custom application, but it's also possible to reproduce with the mdns_responder sample with a couple of changes to the prj.conf to reduce the number of network interfaces (1 IPv6, 1 IPv4) and set CONFIG_NET_SOCKETS_POLL_MAX=3.

You can apply the following patch to reproduce, I have also added an assertion where the failure occurs:

diff --git a/samples/net/mdns_responder/prj.conf b/samples/net/mdns_responder/prj.conf
index c89c505c62a..dba1e628aa0 100644
--- a/samples/net/mdns_responder/prj.conf
+++ b/samples/net/mdns_responder/prj.conf
@@ -5,10 +5,12 @@ CONFIG_NET_IPV6=y
 CONFIG_NET_IPV4=y
 #CONFIG_NET_DHCPV4=y

-CONFIG_NET_IF_MAX_IPV6_COUNT=3
-CONFIG_NET_IF_MAX_IPV4_COUNT=3
+CONFIG_NET_IF_MAX_IPV6_COUNT=1
+CONFIG_NET_IF_MAX_IPV4_COUNT=1

-CONFIG_NET_SOCKETS_POLL_MAX=7
+CONFIG_NET_SOCKETS_POLL_MAX=3
+
+CONFIG_ASSERT=y

 CONFIG_NET_HOSTNAME_ENABLE=y
 CONFIG_NET_HOSTNAME_UNIQUE=n
diff --git a/subsys/net/lib/dns/dispatcher.c b/subsys/net/lib/dns/dispatcher.c
index 674872e65ee..7ba46b05f64 100644
--- a/subsys/net/lib/dns/dispatcher.c
+++ b/subsys/net/lib/dns/dispatcher.c
@@ -287,6 +287,7 @@ int dns_dispatcher_register(struct dns_socket_dispatcher *ctx)
        ctx->pair = NULL;

        for (int i = 0; i < ctx->fds_len; i++) {
+               __ASSERT(ctx->fds[i].fd < ARRAY_SIZE(dispatch_table), "Would access past end of array");
                if (dispatch_table[ctx->fds[i].fd].ctx == NULL) {
                        dispatch_table[ctx->fds[i].fd].ctx = ctx;
                }

I think there are also a couple of other locations in the same file where the out of bounds access could occur, but this one is most obvious because it happens during initialisation.

After applying the patch, you can run:

west build -b native_sim samples/net/mdns_responder -t run

Expected behavior

Read/write is within array bounds

Impact

Undefined behaviour - in my case it overwrites the sockets slist, and then gets stuck in an infinite loop after prepending the ctx to the list (which now already contains ctx). But behaviour could be different depending on what's after the dispatch_table in memory.

Logs and console output

Console output with patch above applied:

WARNING: Using a test - not safe - entropy source
uart connected to pseudotty: /dev/pts/6
ASSERTION FAIL [ctx->fds[i].fd < ((size_t) (((int) sizeof(char[1 - 2 * !(!__builtin_types_compatible_p(__typeof__(dispatch_table), __typeof__(&(dispatch_table)[0])))]) - 1) + (sizeof(dispatch_table) / sizeof((dispatch_table)[0]))))] @ WEST_TOPDIR/zephyr/s
ubsys/net/lib/dns/dispatcher.c:290
    Would access past end of array
@ WEST_TOPDIR/zephyr/lib/os/assert.c:43
[00:00:00.000,000] <err> os: >>> ZEPHYR FATAL ERROR 4: Kernel panic on CPU 0
[00:00:00.000,000] <err> os: Current thread: 0x809cb00 (main)
[00:00:00.000,000] <err> os: Halting system
Exiting due to fatal error

Environment (please complete the following information):

Linux, Zephyr SDK, latest main (commit 57a8a7c3500)

jukkar commented 1 day ago

Interesting side effect, if one also enables CONFIG_DNS_RESOLVER=y, then there is a build assert for too low CONFIG_NET_SOCKETS_POLL_MAX that will prevent compilation.