blobmsg_check_attr_safe adds a length limit specifying the max offset from attr that can be read safely
Signed-off-by: Tobias Schramm <[email protected]> --- blobmsg.c | 27 ++++++++++++++++++++++----- blobmsg.h | 18 ++++++++++++++++++ 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/blobmsg.c b/blobmsg.c index 8019c45..dd4b506 100644 --- a/blobmsg.c +++ b/blobmsg.c @@ -32,18 +32,33 @@ blobmsg_namelen(const struct blobmsg_hdr *hdr) } bool blobmsg_check_attr(const struct blob_attr *attr, bool name) +{ + return blobmsg_check_attr_safe(attr, name, blob_raw_len(attr)); +} + +bool blobmsg_check_attr_safe(const struct blob_attr *attr, bool name, size_t len) { const struct blobmsg_hdr *hdr; const char *data; - int id, len; + char *limit = (char*)attr + len; + int id, data_len; + + if (len < sizeof(struct blob_attr)) + return false; if (blob_len(attr) < sizeof(struct blobmsg_hdr)) return false; + if (len < sizeof(struct blobmsg_hdr)) + return false; + hdr = (void *) attr->data; if (!hdr->namelen && name) return false; + if ((char*)hdr->name + blobmsg_namelen(hdr) > limit) + return false; + if (blobmsg_namelen(hdr) > blob_len(attr) - sizeof(struct blobmsg_hdr)) return false; @@ -51,8 +66,10 @@ bool blobmsg_check_attr(const struct blob_attr *attr, bool name) return false; id = blob_id(attr); - len = blobmsg_data_len(attr); + data_len = blobmsg_data_len(attr); data = blobmsg_data(attr); + if (data_len < 0 || data + data_len > limit) + return false; if (id > BLOBMSG_TYPE_LAST) return false; @@ -60,7 +77,7 @@ bool blobmsg_check_attr(const struct blob_attr *attr, bool name) if (!blob_type[id]) return true; - return blob_check_type(data, len, blob_type[id]); + return blob_check_type(data, data_len, blob_type[id]); } int blobmsg_check_array(const struct blob_attr *attr, int type) @@ -111,7 +128,7 @@ int blobmsg_parse_array(const struct blobmsg_policy *policy, int policy_len, blob_id(attr) != policy[i].type) continue; - if (!blobmsg_check_attr(attr, false)) + if (!blobmsg_check_attr_safe(attr, false, len)) return -1; if (tb[i]) @@ -158,7 +175,7 @@ int blobmsg_parse(const struct blobmsg_policy *policy, int policy_len, if (blobmsg_namelen(hdr) != pslen[i]) continue; - if (!blobmsg_check_attr(attr, true)) + if (!blobmsg_check_attr_safe(attr, true, len)) return -1; if (tb[i]) diff --git a/blobmsg.h b/blobmsg.h index c75f1d9..b1dec4e 100644 --- a/blobmsg.h +++ b/blobmsg.h @@ -104,7 +104,25 @@ static inline int blobmsg_len(const struct blob_attr *attr) return blobmsg_data_len(attr); } +/* + * blobmsg_check_attr: validate a single attribute + * + * This methods may be used with trusted data only. Providing + * malformed blobs will cause out of bounds memory access and + * crash your program or get your device 0wned. + */ bool blobmsg_check_attr(const struct blob_attr *attr, bool name); + +/* + * blobmsg_check_attr_safe: safely validate a single untrusted attribute + * + * This methods is a safe implementation of blobmsg_check_attr. + * It will limit all memory access performed on the blob to the + * range [attr, attr + len] (upper bound non inclusive) and is + * thus suited for checking untrusted blob attributes. + */ +bool blobmsg_check_attr_safe(const struct blob_attr *attr, bool name, size_t len); + bool blobmsg_check_attr_list(const struct blob_attr *attr, int type); /* -- 2.19.1 _______________________________________________ openwrt-devel mailing list [email protected] https://lists.openwrt.org/mailman/listinfo/openwrt-devel
