ubusd: protect against too-short messages
authorJulian Squires <julian@cipht.net>
Wed, 21 Apr 2021 13:36:06 +0000 (11:06 -0230)
committerPetr Štetiar <ynezz@true.cz>
Thu, 3 Jun 2021 08:01:35 +0000 (10:01 +0200)
A bad client can send a message whose blob_attr len is less than 4,
and ubus_msg_new happily points ->data off the end of the allocated
buffer, leading to invalid reads, writes, and eventually a crash if
ubus monitor is running:

==17683== Invalid write of size 4
==17683==    at 0x10A915: client_cb (ubusd_main.c:143)
==17683==    by 0x48495E3: uloop_run_events (uloop.c:198)
==17683==    by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683==    by 0x10A503: uloop_run (uloop.h:111)
==17683==    by 0x10A503: main (ubusd_main.c:284)
==17683==  Address 0x4a63200 is 0 bytes after a block of size 32 alloc'd
==17683==    at 0x4837B65: calloc (vg_replace_malloc.c:752)
==17683==    by 0x10AA87: ubus_msg_new (ubusd.c:47)
==17683==    by 0x10A8CE: client_cb (ubusd_main.c:135)
==17683==    by 0x48495E3: uloop_run_events (uloop.c:198)
==17683==    by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683==    by 0x10A503: uloop_run (uloop.h:111)
==17683==    by 0x10A503: main (ubusd_main.c:284)
==17683==
==17683== Invalid read of size 4
==17683==    at 0x10A645: blob_len (blob.h:102)
==17683==    by 0x10A93D: blob_raw_len (blob.h:111)
==17683==    by 0x10A93D: client_cb (ubusd_main.c:149)
==17683==    by 0x48495E3: uloop_run_events (uloop.c:198)
==17683==    by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683==    by 0x10A503: uloop_run (uloop.h:111)
==17683==    by 0x10A503: main (ubusd_main.c:284)
==17683==  Address 0x4a63200 is 0 bytes after a block of size 32 alloc'd
==17683==    at 0x4837B65: calloc (vg_replace_malloc.c:752)
==17683==    by 0x10AA87: ubus_msg_new (ubusd.c:47)
==17683==    by 0x10A8CE: client_cb (ubusd_main.c:135)
==17683==    by 0x48495E3: uloop_run_events (uloop.c:198)
==17683==    by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683==    by 0x10A503: uloop_run (uloop.h:111)
==17683==    by 0x10A503: main (ubusd_main.c:284)
==17683==
==17683== Invalid read of size 4
==17683==    at 0x10ACE8: blob_len (blob.h:102)
==17683==    by 0x10B7E1: blob_raw_len (blob.h:111)
==17683==    by 0x10B7E1: ubusd_proto_receive_message (ubusd_proto.c:457)
==17683==    by 0x10A9A7: client_cb (ubusd_main.c:169)
==17683==    by 0x48495E3: uloop_run_events (uloop.c:198)
==17683==    by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683==    by 0x10A503: uloop_run (uloop.h:111)
==17683==    by 0x10A503: main (ubusd_main.c:284)
==17683==  Address 0x4a63200 is 0 bytes after a block of size 32 alloc'd
==17683==    at 0x4837B65: calloc (vg_replace_malloc.c:752)
==17683==    by 0x10AA87: ubus_msg_new (ubusd.c:47)
==17683==    by 0x10A8CE: client_cb (ubusd_main.c:135)
==17683==    by 0x48495E3: uloop_run_events (uloop.c:198)
==17683==    by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683==    by 0x10A503: uloop_run (uloop.h:111)
==17683==    by 0x10A503: main (ubusd_main.c:284)
==17683==
==17683== Invalid read of size 4
==17683==    at 0x10D39B: blob_len (blob.h:102)
==17683==    by 0x10D53E: ubusd_monitor_message (ubusd_monitor.c:91)
==17683==    by 0x10A99C: client_cb (ubusd_main.c:168)
==17683==    by 0x48495E3: uloop_run_events (uloop.c:198)
==17683==    by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683==    by 0x10A503: uloop_run (uloop.h:111)
==17683==    by 0x10A503: main (ubusd_main.c:284)
==17683==  Address 0x4a6b3e0 is 0 bytes after a block of size 32 alloc'd
==17683==    at 0x4837B65: calloc (vg_replace_malloc.c:752)
==17683==    by 0x10AA87: ubus_msg_new (ubusd.c:47)
==17683==    by 0x10A8CE: client_cb (ubusd_main.c:135)
==17683==    by 0x48495E3: uloop_run_events (uloop.c:198)
==17683==    by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683==    by 0x10A503: uloop_run (uloop.h:111)
==17683==    by 0x10A503: main (ubusd_main.c:284)
==17683==
==17683== Invalid read of size 1
==17683==    at 0x4848286: blob_put (blob.c:167)
==17683==    by 0x10D555: ubusd_monitor_message (ubusd_monitor.c:91)
==17683==    by 0x10A99C: client_cb (ubusd_main.c:168)
==17683==    by 0x48495E3: uloop_run_events (uloop.c:198)
==17683==    by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683==    by 0x10A503: uloop_run (uloop.h:111)
==17683==    by 0x10A503: main (ubusd_main.c:284)
==17683==  Address 0x4a6b3e4 is 4 bytes after a block of size 32 alloc'd
==17683==    at 0x4837B65: calloc (vg_replace_malloc.c:752)
==17683==    by 0x10AA87: ubus_msg_new (ubusd.c:47)
==17683==    by 0x10A8CE: client_cb (ubusd_main.c:135)
==17683==    by 0x48495E3: uloop_run_events (uloop.c:198)
==17683==    by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683==    by 0x10A503: uloop_run (uloop.h:111)
==17683==    by 0x10A503: main (ubusd_main.c:284)
==17683==
==17683==
==17683== Process terminating with default action of signal 11 (SIGSEGV)
==17683==  Bad permissions for mapped region at address 0x4E43000
==17683==    at 0x4848286: blob_put (blob.c:167)
==17683==    by 0x10D555: ubusd_monitor_message (ubusd_monitor.c:91)
==17683==    by 0x10A99C: client_cb (ubusd_main.c:168)
==17683==    by 0x48495E3: uloop_run_events (uloop.c:198)
==17683==    by 0x48495E3: uloop_run_timeout (uloop.c:555)
==17683==    by 0x10A503: uloop_run (uloop.h:111)
==17683==    by 0x10A503: main (ubusd_main.c:284)

The following Python program minimally reproduces the issue:

import socket

sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
sock.connect('/tmp/usock')
sock.recv(12)
sock.send(b'\x00\x04\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00')

Signed-off-by: Julian Squires <julian@cipht.net>
(cherry picked from commit a8cf678230ed163ff7a07eb1e2c872f9d655460a)

ubusd_main.c

index d3c12f403067246936733627ab11e48253358ab4..3acb29d0e905f4f58d9db1f72b1d8f64d313c539 100644 (file)
@@ -113,6 +113,8 @@ retry:
                if (cl->pending_msg_offset < (int) sizeof(cl->hdrbuf))
                        goto out;
 
                if (cl->pending_msg_offset < (int) sizeof(cl->hdrbuf))
                        goto out;
 
+               if (blob_raw_len(&cl->hdrbuf.data) < sizeof(struct blob_attr))
+                       goto disconnect;
                if (blob_pad_len(&cl->hdrbuf.data) > UBUS_MAX_MSGLEN)
                        goto disconnect;
 
                if (blob_pad_len(&cl->hdrbuf.data) > UBUS_MAX_MSGLEN)
                        goto disconnect;