fix blob parsing vulnerability by using blob_parse_untrusted

blob_parse expects blobs from trusted inputs, but it can be supplied
with possibly malicious blobs from untrusted inputs as well, which might
lead to undefined behaviour and/or crash of ubus daemon. In order to
prevent such conditions, switch to blob_parse_untrusted which should
hopefully handle such untrusted inputs appropriately.

Signed-off-by: Petr Štetiar <ynezz@true.cz>

diff --git a/cli.c b/cli.c
index 421f244ad9dcf383253e21d602b5eb79eb85f8fd..f566279b4f6d1b54de85985518365167731cba13 100644 (file)
--- a/cli.c
+++ b/cli.c
@@ -472,7 +472,7 @@ ubus_cli_monitor_cb(struct ubus_context *ctx, uint32_t seq, struct blob_attr *ms
        bool send;
        char *data;
 
-       blob_parse(msg, tb, policy, UBUS_MONITOR_MAX);
+       blob_parse_untrusted(msg, blob_raw_len(msg), tb, policy, UBUS_MONITOR_MAX);
 
        if (!tb[UBUS_MONITOR_CLIENT] ||
            !tb[UBUS_MONITOR_PEER] ||

diff --git a/libubus-internal.h b/libubus-internal.h
index 8cf99b3bc6b12fbab467a719c9098bb4117d8214..24477a03214421c12016fb209b64e2549f1717b5 100644 (file)
--- a/libubus-internal.h
+++ b/libubus-internal.h
@@ -17,7 +17,7 @@
 extern struct blob_buf b;
 extern const struct ubus_method watch_method;
 
-struct blob_attr **ubus_parse_msg(struct blob_attr *msg);
+struct blob_attr **ubus_parse_msg(struct blob_attr *msg, size_t len);
 bool ubus_validate_hdr(struct ubus_msghdr *hdr);
 void ubus_handle_data(struct uloop_fd *u, unsigned int events);
 int ubus_send_msg(struct ubus_context *ctx, uint32_t seq,

diff --git a/libubus-io.c b/libubus-io.c
index 81c1cd1309b11d8fe905a550c3f37fcbc45667dd..ba1016d0fa09b5cd7081209545f6a34e7104c853 100644 (file)
--- a/libubus-io.c
+++ b/libubus-io.c
@@ -43,9 +43,9 @@ static const struct blob_attr_info ubus_policy[UBUS_ATTR_MAX] = {
 
 static struct blob_attr *attrbuf[UBUS_ATTR_MAX];
 
-__hidden struct blob_attr **ubus_parse_msg(struct blob_attr *msg)
+__hidden struct blob_attr **ubus_parse_msg(struct blob_attr *msg, size_t len)
 {
-       blob_parse(msg, attrbuf, ubus_policy, UBUS_ATTR_MAX);
+       blob_parse_untrusted(msg, len, attrbuf, ubus_policy, UBUS_ATTR_MAX);
        return attrbuf;
 }
 

diff --git a/libubus-obj.c b/libubus-obj.c
index 2580b2442c4b40ff26f0f7165a966ab9db615b4a..29cbb2b98e6eab48d8cd280e6ca2a596da1ff729 100644 (file)
--- a/libubus-obj.c
+++ b/libubus-obj.c
@@ -121,7 +121,7 @@ void __hidden ubus_process_obj_msg(struct ubus_context *ctx, struct ubus_msghdr_
        struct ubus_object *obj;
        uint32_t objid;
        void *prev_data = NULL;
-       attrbuf = ubus_parse_msg(buf->data);
+       attrbuf = ubus_parse_msg(buf->data, blob_raw_len(buf->data));
        if (!attrbuf[UBUS_ATTR_OBJID])
                return;
 
@@ -160,7 +160,7 @@ void __hidden ubus_process_obj_msg(struct ubus_context *ctx, struct ubus_msghdr_
 static void ubus_add_object_cb(struct ubus_request *req, int type, struct blob_attr *msg)
 {
        struct ubus_object *obj = req->priv;
-       struct blob_attr **attrbuf = ubus_parse_msg(msg);
+       struct blob_attr **attrbuf = ubus_parse_msg(msg, blob_raw_len(msg));
 
        if (!attrbuf[UBUS_ATTR_OBJID])
                return;
@@ -240,7 +240,7 @@ int ubus_add_object(struct ubus_context *ctx, struct ubus_object *obj)
 static void ubus_remove_object_cb(struct ubus_request *req, int type, struct blob_attr *msg)
 {
        struct ubus_object *obj = req->priv;
-       struct blob_attr **attrbuf = ubus_parse_msg(msg);
+       struct blob_attr **attrbuf = ubus_parse_msg(msg, blob_raw_len(msg));
 
        if (!attrbuf[UBUS_ATTR_OBJID])
                return;

diff --git a/libubus-req.c b/libubus-req.c
index fd9a548839e45ae0180f7bf09129bcdd6f9f845b..ae9d1925ecdf1845f5d536b50240216f9ef56fb4 100644 (file)
--- a/libubus-req.c
+++ b/libubus-req.c
@@ -31,7 +31,7 @@ static void req_data_cb(struct ubus_request *req, int type, struct blob_attr *da
        if (!req->data_cb)
                return;
 
-       attr = ubus_parse_msg(data);
+       attr = ubus_parse_msg(data, blob_raw_len(data));
        if (!attr[UBUS_ATTR_DATA])
                return;
 
@@ -328,7 +328,7 @@ int ubus_notify(struct ubus_context *ctx, struct ubus_object *obj,
 
 static bool ubus_get_status(struct ubus_msghdr_buf *buf, int *ret)
 {
-       struct blob_attr **attrbuf = ubus_parse_msg(buf->data);
+       struct blob_attr **attrbuf = ubus_parse_msg(buf->data, blob_raw_len(buf->data));
 
        if (!attrbuf[UBUS_ATTR_STATUS])
                return false;
@@ -435,7 +435,7 @@ static void ubus_process_notify_status(struct ubus_request *req, int id, struct
 
        if (!id) {
                /* first id: ubusd's status message with a list of ids */
-               tb = ubus_parse_msg(buf->data);
+               tb = ubus_parse_msg(buf->data, blob_raw_len(buf->data));
                if (tb[UBUS_ATTR_SUBSCRIBERS]) {
                        blob_for_each_attr(cur, tb[UBUS_ATTR_SUBSCRIBERS], rem) {
                                if (!blob_check_type(blob_data(cur), blob_len(cur), BLOB_ATTR_INT32))

diff --git a/libubus.c b/libubus.c
index b405891416c22856729ae2193cde60231bd52ad3..91f317c59867e57005618c3a94d835ff8d1d81fb 100644 (file)
--- a/libubus.c
+++ b/libubus.c
@@ -139,7 +139,7 @@ static void ubus_lookup_cb(struct ubus_request *ureq, int type, struct blob_attr
        struct blob_attr **attr;
 
        req = container_of(ureq, struct ubus_lookup_request, req);
-       attr = ubus_parse_msg(msg);
+       attr = ubus_parse_msg(msg, blob_raw_len(msg));
 
        if (!attr[UBUS_ATTR_OBJID] || !attr[UBUS_ATTR_OBJPATH] ||
            !attr[UBUS_ATTR_OBJTYPE])
@@ -175,7 +175,7 @@ static void ubus_lookup_id_cb(struct ubus_request *req, int type, struct blob_at
        struct blob_attr **attr;
        uint32_t *id = req->priv;
 
-       attr = ubus_parse_msg(msg);
+       attr = ubus_parse_msg(msg, blob_raw_len(msg));
 
        if (!attr[UBUS_ATTR_OBJID])
                return;

diff --git a/tests/fuzz/test-fuzz.c b/tests/fuzz/test-fuzz.c
index 9922ff9de609d6d4096c0e56137c2c674c3c8bdb..7a7a1ebe8b11c5a2d0fc997309a9d39aab72f7fd 100644 (file)
--- a/tests/fuzz/test-fuzz.c
+++ b/tests/fuzz/test-fuzz.c
@@ -28,7 +28,7 @@ static void _ubus_parse_msg(const uint8_t *data, size_t size)
        if (blob_pad_len(attr) > UBUS_MAX_MSGLEN)
                return;
 
-       ubus_parse_msg(attr);
+       ubus_parse_msg(attr, size);
 }
 
 int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)

diff --git a/ubusd.h b/ubusd.h
index 867cde9a384bacacec2a39ec366ca8abbd670aba..923e43dd92da313243dde29cb23bc1dc2c0b4b08 100644 (file)
--- a/ubusd.h
+++ b/ubusd.h
@@ -72,7 +72,7 @@ struct ubus_msg_buf *ubus_msg_new(void *data, int len, bool shared);
 void ubus_msg_send(struct ubus_client *cl, struct ubus_msg_buf *ub);
 ssize_t ubus_msg_writev(int fd, struct ubus_msg_buf *ub, size_t offset);
 void ubus_msg_free(struct ubus_msg_buf *ub);
-struct blob_attr **ubus_parse_msg(struct blob_attr *msg);
+struct blob_attr **ubus_parse_msg(struct blob_attr *msg, size_t len);
 
 struct ubus_client *ubusd_proto_new_client(int fd, uloop_fd_handler cb);
 void ubusd_proto_receive_message(struct ubus_client *cl, struct ubus_msg_buf *ub);

diff --git a/ubusd_acl.c b/ubusd_acl.c
index f19df9a875c7671257d5c3d4a582edb5371f42a6..e426a4af95efaf1e610bf640013dd8b7978ec015 100644 (file)
--- a/ubusd_acl.c
+++ b/ubusd_acl.c
@@ -549,7 +549,7 @@ static int ubusd_reply_query(struct ubus_client *cl, struct ubus_msg_buf *ub, st
 static int ubusd_acl_recv(struct ubus_client *cl, struct ubus_msg_buf *ub, const char *method, struct blob_attr *msg)
 {
        if (!strcmp(method, "query"))
-               return ubusd_reply_query(cl, ub, ubus_parse_msg(ub->data), msg);
+               return ubusd_reply_query(cl, ub, ubus_parse_msg(ub->data, blob_raw_len(ub->data)), msg);
 
        return UBUS_STATUS_INVALID_COMMAND;
 }

diff --git a/ubusd_proto.c b/ubusd_proto.c
index 4dd89ddb4939d6acb706fbbea1467a9e56d6c1f4..4746605f4960a45145741919edb49535f032dfe8 100644 (file)
--- a/ubusd_proto.c
+++ b/ubusd_proto.c
@@ -34,9 +34,9 @@ static const struct blob_attr_info ubus_policy[UBUS_ATTR_MAX] = {
        [UBUS_ATTR_GROUP] = { .type = BLOB_ATTR_STRING },
 };
 
-struct blob_attr **ubus_parse_msg(struct blob_attr *msg)
+struct blob_attr **ubus_parse_msg(struct blob_attr *msg, size_t len)
 {
-       blob_parse(msg, attrbuf, ubus_policy, UBUS_ATTR_MAX);
+       blob_parse_untrusted(msg, len, attrbuf, ubus_policy, UBUS_ATTR_MAX);
        return attrbuf;
 }
 
@@ -454,7 +454,7 @@ void ubusd_proto_receive_message(struct ubus_client *cl, struct ubus_msg_buf *ub
        /* Note: no callback should free the `ub` buffer
                 that's always done right after the callback finishes */
        if (cb)
-               ret = cb(cl, ub, ubus_parse_msg(ub->data));
+               ret = cb(cl, ub, ubus_parse_msg(ub->data, blob_raw_len(ub->data)));
        else
                ret = UBUS_STATUS_INVALID_COMMAND;