diff mbox series

[OpenWrt-Devel,libubox,18/20] blobmsg: add _len variants for all attribute checking methods

Message ID 20191219215836.21773-19-ynezz@true.cz
State Accepted
Delegated to: Petr Štetiar
Headers show
Series tests, fuzzing, fixes and improvements | expand

Commit Message

Petr Štetiar Dec. 19, 2019, 9:58 p.m. UTC 
From: Tobias Schramm <tobleminer@gmail.com>

Introduce _len variants of blobmsg attribute checking functions which
aims to provide safer implementation as those functions should limit all
memory accesses performed on the blob to the range [attr, attr + len]
(upper bound non inclusive) and thus should be suited for checking of
untrusted blob attributes.

While at it add some comments in order to make it clear.

Signed-off-by: Tobias Schramm <tobleminer@gmail.com>
[_safe -> _len, blobmsg_check_array_len fix, commit subject/desc facelift]
Signed-off-by: Petr Štetiar <ynezz@true.cz>
---
 blobmsg.c | 21 ++++++++++++++++++---
 blobmsg.h | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 72 insertions(+), 4 deletions(-)
diff mbox series

Patch

diff --git a/blobmsg.c b/blobmsg.c
index fbc6d2de9135..7cd0934600de 100644
--- a/blobmsg.c
+++ b/blobmsg.c
@@ -100,12 +100,22 @@  bool blobmsg_check_attr_len(const struct blob_attr *attr, bool name, size_t len)
 }
 
 int blobmsg_check_array(const struct blob_attr *attr, int type)
+{
+	return blobmsg_check_array_len(attr, type, blob_raw_len(attr));
+}
+
+int blobmsg_check_array_len(const struct blob_attr *attr, int type, size_t len)
 {
 	struct blob_attr *cur;
 	bool name;
-	size_t rem;
 	int size = 0;
 
+	if (type > BLOBMSG_TYPE_LAST)
+		return -1;
+
+	if (!blobmsg_check_attr_len(attr, false, len))
+		return -1;
+
 	switch (blobmsg_type(attr)) {
 	case BLOBMSG_TYPE_TABLE:
 		name = true;
@@ -117,11 +127,11 @@  int blobmsg_check_array(const struct blob_attr *attr, int type)
 		return -1;
 	}
 
-	blobmsg_for_each_attr(cur, attr, rem) {
+	__blobmsg_for_each_attr(cur, attr, len) {
 		if (type != BLOBMSG_TYPE_UNSPEC && blobmsg_type(cur) != type)
 			return -1;
 
-		if (!blobmsg_check_attr(cur, name))
+		if (!blobmsg_check_attr_len(cur, name, len))
 			return -1;
 
 		size++;
@@ -135,6 +145,11 @@  bool blobmsg_check_attr_list(const struct blob_attr *attr, int type)
 	return blobmsg_check_array(attr, type) >= 0;
 }
 
+bool blobmsg_check_attr_list_len(const struct blob_attr *attr, int type, size_t len)
+{
+	return blobmsg_check_array_len(attr, type, len) >= 0;
+}
+
 int blobmsg_parse_array(const struct blobmsg_policy *policy, int policy_len,
 			struct blob_attr **tb, void *data, unsigned int len)
 {
diff --git a/blobmsg.h b/blobmsg.h
index c44015942a37..af88c1feb86f 100644
--- a/blobmsg.h
+++ b/blobmsg.h
@@ -104,19 +104,66 @@  static inline size_t blobmsg_len(const struct blob_attr *attr)
 	return blobmsg_data_len(attr);
 }
 
+/*
+ * blobmsg_check_attr: validate a list of attributes
+ *
+ * This method may be used with trusted data only. Providing
+ * malformed blobs will cause out of bounds memory access.
+ */
 bool blobmsg_check_attr(const struct blob_attr *attr, bool name);
-bool blobmsg_check_attr_list(const struct blob_attr *attr, int type);
 
+/*
+ * blobmsg_check_attr_len: validate a list of attributes
+ *
+ * This method should be safer 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 of untrusted blob attributes.
+ */
 bool blobmsg_check_attr_len(const struct blob_attr *attr, bool name, size_t len);
 
+/*
+ * blobmsg_check_attr_list: validate a list of attributes
+ *
+ * This method may be used with trusted data only. Providing
+ * malformed blobs will cause out of bounds memory access.
+ */
+bool blobmsg_check_attr_list(const struct blob_attr *attr, int type);
+
+/*
+ * blobmsg_check_attr_list_len: validate a list of untrusted attributes
+ *
+ * This method should be safer implementation of blobmsg_check_attr_list.
+ * 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 of untrusted blob attributes.
+ */
+bool blobmsg_check_attr_list_len(const struct blob_attr *attr, int type, size_t len);
+
 /*
  * blobmsg_check_array: validate array/table and return size
  *
  * Checks if all elements of an array or table are valid and have
  * the specified type. Returns the number of elements in the array
+ *
+ * This method may be used with trusted data only. Providing
+ * malformed blobs will cause out of bounds memory access.
  */
 int blobmsg_check_array(const struct blob_attr *attr, int type);
 
+/*
+ * blobmsg_check_array_len: validate untrusted array/table and return size
+ *
+ * Checks if all elements of an array or table are valid and have
+ * the specified type. Returns the number of elements in the array.
+ *
+ * This method should be safer implementation of blobmsg_check_array.
+ * 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 of untrusted blob attributes.
+ */
+int blobmsg_check_array_len(const struct blob_attr *attr, int type, size_t len);
+
 int blobmsg_parse(const struct blobmsg_policy *policy, int policy_len,
                   struct blob_attr **tb, void *data, unsigned int len);
 int blobmsg_parse_array(const struct blobmsg_policy *policy, int policy_len,
@@ -272,4 +319,10 @@  int blobmsg_printf(struct blob_buf *buf, const char *name, const char *format, .
 	     (blob_pad_len(pos) >= sizeof(struct blob_attr)); \
 	     rem -= blob_pad_len(pos), pos = blob_next(pos))
 
+#define __blobmsg_for_each_attr(pos, attr, rem) \
+	for (pos = (struct blob_attr *) (attr ? blobmsg_data(attr) : NULL); \
+	     rem >= sizeof(struct blob_attr) && (blob_pad_len(pos) <= rem) && \
+	     (blob_pad_len(pos) >= sizeof(struct blob_attr)); \
+	     rem -= blob_pad_len(pos), pos = blob_next(pos))
+
 #endif