blobmsg_check_attr_safe adds a length limit specifying the max offset from attr that can be read safely
Signed-off-by: Tobias Schramm <toblemi...@gmail.com> --- blobmsg.c | 24 ++++++++++++++++++------ blobmsg.h | 24 +++++++++++++++++++++++- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/blobmsg.c b/blobmsg.c index 8019c45..10f3801 100644 --- a/blobmsg.c +++ b/blobmsg.c @@ -31,19 +31,29 @@ blobmsg_namelen(const struct blobmsg_hdr *hdr) return be16_to_cpu(hdr->namelen); } -bool blobmsg_check_attr(const struct blob_attr *attr, bool name) +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 +61,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 +72,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 +123,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 +170,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..d17b896 100644 --- a/blobmsg.h +++ b/blobmsg.h @@ -104,7 +104,29 @@ static inline int blobmsg_len(const struct blob_attr *attr) return blobmsg_data_len(attr); } -bool blobmsg_check_attr(const struct blob_attr *attr, bool name); +/* + * blobmsg_check_attr_safe: safely validate a single untrusted attribute + * + * This method 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); + +/* + * blobmsg_check_attr: validate a single attribute + * + * This method 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. + */ +static inline 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_list(const struct blob_attr *attr, int type); /* -- 2.19.2 _______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel