[PATCH] ubusd: protect against too-short messages

[Julian Squires]

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 at cipht.net>
---
 ubusd_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ubusd_main.c b/ubusd_main.c
index c3d8049..9fd0967 100644
--- a/ubusd_main.c
+++ b/ubusd_main.c
@@ -129,6 +129,8 @@ retry:
 		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;
 
-- 
2.31.1