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>
This commit is contained in:
Petr Štetiar 2019-12-19 11:25:56 +01:00
parent c60583743c
commit a1523d76b0
10 changed files with 18 additions and 18 deletions

2
cli.c
View file

@ -472,7 +472,7 @@ ubus_cli_monitor_cb(struct ubus_context *ctx, uint32_t seq, struct blob_attr *ms
bool send; bool send;
char *data; 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] || if (!tb[UBUS_MONITOR_CLIENT] ||
!tb[UBUS_MONITOR_PEER] || !tb[UBUS_MONITOR_PEER] ||

View file

@ -17,7 +17,7 @@
extern struct blob_buf b; extern struct blob_buf b;
extern const struct ubus_method watch_method; 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); bool ubus_validate_hdr(struct ubus_msghdr *hdr);
void ubus_handle_data(struct uloop_fd *u, unsigned int events); void ubus_handle_data(struct uloop_fd *u, unsigned int events);
int ubus_send_msg(struct ubus_context *ctx, uint32_t seq, int ubus_send_msg(struct ubus_context *ctx, uint32_t seq,

View file

@ -43,9 +43,9 @@ static const struct blob_attr_info ubus_policy[UBUS_ATTR_MAX] = {
static struct blob_attr *attrbuf[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; return attrbuf;
} }

View file

@ -121,7 +121,7 @@ void __hidden ubus_process_obj_msg(struct ubus_context *ctx, struct ubus_msghdr_
struct ubus_object *obj; struct ubus_object *obj;
uint32_t objid; uint32_t objid;
void *prev_data = NULL; 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]) if (!attrbuf[UBUS_ATTR_OBJID])
return; 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) static void ubus_add_object_cb(struct ubus_request *req, int type, struct blob_attr *msg)
{ {
struct ubus_object *obj = req->priv; 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]) if (!attrbuf[UBUS_ATTR_OBJID])
return; 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) static void ubus_remove_object_cb(struct ubus_request *req, int type, struct blob_attr *msg)
{ {
struct ubus_object *obj = req->priv; 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]) if (!attrbuf[UBUS_ATTR_OBJID])
return; return;

View file

@ -31,7 +31,7 @@ static void req_data_cb(struct ubus_request *req, int type, struct blob_attr *da
if (!req->data_cb) if (!req->data_cb)
return; return;
attr = ubus_parse_msg(data); attr = ubus_parse_msg(data, blob_raw_len(data));
if (!attr[UBUS_ATTR_DATA]) if (!attr[UBUS_ATTR_DATA])
return; 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) 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]) if (!attrbuf[UBUS_ATTR_STATUS])
return false; return false;
@ -435,7 +435,7 @@ static void ubus_process_notify_status(struct ubus_request *req, int id, struct
if (!id) { if (!id) {
/* first id: ubusd's status message with a list of ids */ /* 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]) { if (tb[UBUS_ATTR_SUBSCRIBERS]) {
blob_for_each_attr(cur, tb[UBUS_ATTR_SUBSCRIBERS], rem) { blob_for_each_attr(cur, tb[UBUS_ATTR_SUBSCRIBERS], rem) {
if (!blob_check_type(blob_data(cur), blob_len(cur), BLOB_ATTR_INT32)) if (!blob_check_type(blob_data(cur), blob_len(cur), BLOB_ATTR_INT32))

View file

@ -139,7 +139,7 @@ static void ubus_lookup_cb(struct ubus_request *ureq, int type, struct blob_attr
struct blob_attr **attr; struct blob_attr **attr;
req = container_of(ureq, struct ubus_lookup_request, req); 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] || if (!attr[UBUS_ATTR_OBJID] || !attr[UBUS_ATTR_OBJPATH] ||
!attr[UBUS_ATTR_OBJTYPE]) !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; struct blob_attr **attr;
uint32_t *id = req->priv; uint32_t *id = req->priv;
attr = ubus_parse_msg(msg); attr = ubus_parse_msg(msg, blob_raw_len(msg));
if (!attr[UBUS_ATTR_OBJID]) if (!attr[UBUS_ATTR_OBJID])
return; return;

View file

@ -28,7 +28,7 @@ static void _ubus_parse_msg(const uint8_t *data, size_t size)
if (blob_pad_len(attr) > UBUS_MAX_MSGLEN) if (blob_pad_len(attr) > UBUS_MAX_MSGLEN)
return; return;
ubus_parse_msg(attr); ubus_parse_msg(attr, size);
} }
int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)

View file

@ -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); 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); ssize_t ubus_msg_writev(int fd, struct ubus_msg_buf *ub, size_t offset);
void ubus_msg_free(struct ubus_msg_buf *ub); 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); 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); void ubusd_proto_receive_message(struct ubus_client *cl, struct ubus_msg_buf *ub);

View file

@ -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) 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")) 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; return UBUS_STATUS_INVALID_COMMAND;
} }

View file

@ -34,9 +34,9 @@ static const struct blob_attr_info ubus_policy[UBUS_ATTR_MAX] = {
[UBUS_ATTR_GROUP] = { .type = BLOB_ATTR_STRING }, [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; 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 /* Note: no callback should free the `ub` buffer
that's always done right after the callback finishes */ that's always done right after the callback finishes */
if (cb) 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 else
ret = UBUS_STATUS_INVALID_COMMAND; ret = UBUS_STATUS_INVALID_COMMAND;