mirror
/
openwrt
mirror of https://git.openwrt.org/openwrt/openwrt.git
1
0
Fork 0
Code Releases Activity

libubox: backport additional length-checking fixes 

Fixes: FS#3177
Cc: Felix Fietkau <nbd@nbd.name>
Cc: Rafał Miłecki <rafal@milecki.pl>
Signed-off-by: Baptiste Jonglez <git@bitsofnetworks.org>
This commit is contained in:
Baptiste Jonglez 2020-06-13 20:17:40 +02:00 committed by Hauke Mehrtens
parent 0f07496f52
commit 2dcf46b079
5 changed files with 284 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
package/libs/libubox/Makefile
View File

@ -1,7 +1,7 @@
include $(TOPDIR)/rules.mk
PKG_NAME:=libubox
PKG_RELEASE=4
PKG_RELEASE=5
PKG_SOURCE_PROTO:=git
PKG_SOURCE_URL=$(PROJECT_GIT)/project/libubox.git

73
package/libs/libubox/patches/0018-blobmsg-fix-attrs-iteration-in-the-blobmsg_check_arr.patch Normal file
View File

@ -0,0 +1,73 @@
From 5e75160f48785464f9213c6bc8c72b9372c5318b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= <rafal@milecki.pl>
Date: Sat, 23 May 2020 13:18:51 +0200
Subject: [PATCH] blobmsg: fix attrs iteration in the blobmsg_check_array_len()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Starting with 75e300aeec25 ("blobmsg: fix wrong payload len passed from
blobmsg_check_array") blobmsg_check_array_len() gets *blob* length
passed as argument. It cannot be used with __blobmsg_for_each_attr()
which expects *data* length.
Use blobmsg_for_each_attr() which calculates *data* length on its own.
The same bug was already reported in the past and there was fix attempt
in the commit cd75136b1342 ("blobmsg: fix wrong payload len passed from
blobmsg_check_array"). That change made blobmsg_check_attr_len() calls
fail however.
This is hopefully the correct & complete fix:
1. blobmsg_check_array_len() gets *blob* length
2. It calls blobmsg_check_attr_len() which requires *blob* length
3. It uses blobmsg_for_each_attr() which gets *data* length
This fixes iterating over random memory treated as attrs. That was
resulting in check failing randomly for totally correct blobs. It's
critical e.g. for procd project with its instance_fill_array() failing
and procd not starting services.
Fixes: 75e300aeec25 ("blobmsg: fix wrong payload len passed from blobmsg_check_array")
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
blobmsg.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
--- a/blobmsg.c
+++ b/blobmsg.c
@@ -123,16 +123,18 @@ int blobmsg_check_array(const struct blo
return blobmsg_check_array_len(attr, type, blob_len(attr));
}
-int blobmsg_check_array_len(const struct blob_attr *attr, int type, size_t len)
+int blobmsg_check_array_len(const struct blob_attr *attr, int type,
+ size_t blob_len)
{
struct blob_attr *cur;
+ size_t rem;
bool name;
int size = 0;
if (type > BLOBMSG_TYPE_LAST)
return -1;
- if (!blobmsg_check_attr_len(attr, false, len))
+ if (!blobmsg_check_attr_len(attr, false, blob_len))
return -1;
switch (blobmsg_type(attr)) {
@@ -146,11 +148,11 @@ int blobmsg_check_array_len(const struct
return -1;
}
- __blobmsg_for_each_attr(cur, attr, len) {
+ blobmsg_for_each_attr(cur, attr, rem) {
if (type != BLOBMSG_TYPE_UNSPEC && blobmsg_type(cur) != type)
return -1;
- if (!blobmsg_check_attr_len(cur, name, len))
+ if (!blobmsg_check_attr_len(cur, name, rem))
return -1;
size++;

26
package/libs/libubox/patches/0019-blobmsg-fix-length-in-blobmsg_check_array.patch Normal file
View File

@ -0,0 +1,26 @@
From c2fc622b771f679e8f55060ac60cfe02b9a80995 Mon Sep 17 00:00:00 2001
From: Felix Fietkau <nbd@nbd.name>
Date: Mon, 25 May 2020 13:44:20 +0200
Subject: [PATCH] blobmsg: fix length in blobmsg_check_array
blobmsg_check_array_len expects the length of the full attribute buffer,
not just the data length.
Due to other missing length checks (fixed in the next commit), this did
not show up as a test failure
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
blobmsg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/blobmsg.c
+++ b/blobmsg.c
@@ -120,7 +120,7 @@ bool blobmsg_check_attr_len(const struct
int blobmsg_check_array(const struct blob_attr *attr, int type)
{
- return blobmsg_check_array_len(attr, type, blob_len(attr));
+ return blobmsg_check_array_len(attr, type, blob_raw_len(attr));
}
int blobmsg_check_array_len(const struct blob_attr *attr, int type,

47
package/libs/libubox/patches/0020-blobmsg-simplify-and-fix-name-length-checks-in-blobm.patch Normal file
View File

@ -0,0 +1,47 @@
From 639c29d19717616b809d9a1e9042461ab8024370 Mon Sep 17 00:00:00 2001
From: Felix Fietkau <nbd@nbd.name>
Date: Mon, 25 May 2020 14:49:35 +0200
Subject: [PATCH] blobmsg: simplify and fix name length checks in
blobmsg_check_name
blobmsg_hdr_valid_namelen was omitted when name==false
The blob_len vs blobmsg_namelen changes were not taking into account
potential padding between name and data
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
blobmsg.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
--- a/blobmsg.c
+++ b/blobmsg.c
@@ -54,8 +54,8 @@ static bool blobmsg_hdr_valid_namelen(co
static bool blobmsg_check_name(const struct blob_attr *attr, size_t len, bool name)
{
- char *limit = (char *) attr + len;
const struct blobmsg_hdr *hdr;
+ uint16_t namelen;
hdr = blobmsg_hdr_from_blob(attr, len);
if (!hdr)
@@ -64,16 +64,11 @@ static bool blobmsg_check_name(const str
if (name && !hdr->namelen)
return false;
- if (name && !blobmsg_hdr_valid_namelen(hdr, len))
+ namelen = blobmsg_namelen(hdr);
+ if (blob_len(attr) < (size_t)blobmsg_hdrlen(namelen))
return false;
- if ((char *) hdr->name + blobmsg_namelen(hdr) + 1 > limit)
- return false;
-
- if (blobmsg_namelen(hdr) > (blob_len(attr) - sizeof(struct blobmsg_hdr)))
- return false;
-
- if (hdr->name[blobmsg_namelen(hdr)] != 0)
+ if (hdr->name[namelen] != 0)
return false;
return true;

137
package/libs/libubox/patches/0021-blobmsg-fix-missing-length-checks.patch Normal file
View File

@ -0,0 +1,137 @@
From 66195aee50424cbda0c2d858014e4cc58a2dc029 Mon Sep 17 00:00:00 2001
From: Felix Fietkau <nbd@nbd.name>
Date: Mon, 25 May 2020 12:40:04 +0200
Subject: [PATCH] blobmsg: fix missing length checks
blobmsg_check_attr_len was calling blobmsg_check_data for some, but not all
attribute types. These checks was missing for arrays and tables.
Additionally, the length check in blobmsg_check_data was a bit off, since
it was comparing the blobmsg data length against the raw blob attr length.
Fix this by checking the raw blob length against the buffer length in
blobmsg_hdr_from_blob
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
blobmsg.c | 66 +++++++++++++++++--------------------------------------
1 file changed, 20 insertions(+), 46 deletions(-)
--- a/blobmsg.c
+++ b/blobmsg.c
@@ -36,31 +36,18 @@ bool blobmsg_check_attr(const struct blo
return blobmsg_check_attr_len(attr, name, blob_raw_len(attr));
}
-static const struct blobmsg_hdr* blobmsg_hdr_from_blob(const struct blob_attr *attr, size_t len)
-{
- if (len < sizeof(struct blob_attr) + sizeof(struct blobmsg_hdr))
- return NULL;
-
- return blob_data(attr);
-}
-
-static bool blobmsg_hdr_valid_namelen(const struct blobmsg_hdr *hdr, size_t len)
-{
- if (len < sizeof(struct blob_attr) + sizeof(struct blobmsg_hdr) + blobmsg_namelen(hdr) + 1)
- return false;
-
- return true;
-}
-
-static bool blobmsg_check_name(const struct blob_attr *attr, size_t len, bool name)
+static bool blobmsg_check_name(const struct blob_attr *attr, bool name)
{
const struct blobmsg_hdr *hdr;
uint16_t namelen;
- hdr = blobmsg_hdr_from_blob(attr, len);
- if (!hdr)
+ if (!blob_is_extended(attr))
+ return !name;
+
+ if (blob_len(attr) < sizeof(struct blobmsg_hdr))
return false;
+ hdr = (const struct blobmsg_hdr *)blob_data(attr);
if (name && !hdr->namelen)
return false;
@@ -74,29 +61,20 @@ static bool blobmsg_check_name(const str
return true;
}
-static const char* blobmsg_check_data(const struct blob_attr *attr, size_t len, size_t *data_len)
-{
- char *limit = (char *) attr + len;
- const char *data;
-
- *data_len = blobmsg_data_len(attr);
- if (*data_len > blob_raw_len(attr))
- return NULL;
-
- data = blobmsg_data(attr);
- if (data + *data_len > limit)
- return NULL;
-
- return data;
-}
-
bool blobmsg_check_attr_len(const struct blob_attr *attr, bool name, size_t len)
{
const char *data;
size_t data_len;
int id;
- if (!blobmsg_check_name(attr, len, name))
+ if (len < sizeof(struct blob_attr))
+ return false;
+
+ data_len = blob_raw_len(attr);
+ if (data_len < sizeof(struct blob_attr) || data_len > len)
+ return false;
+
+ if (!blobmsg_check_name(attr, name))
return false;
id = blob_id(attr);
@@ -106,9 +84,8 @@ bool blobmsg_check_attr_len(const struct
if (!blob_type[id])
return true;
- data = blobmsg_check_data(attr, len, &data_len);
- if (!data)
- return false;
+ data = blobmsg_data(attr);
+ data_len = blobmsg_data_len(attr);
return blob_check_type(data, data_len, blob_type[id]);
}
@@ -212,13 +189,13 @@ int blobmsg_parse(const struct blobmsg_p
}
__blob_for_each_attr(attr, data, len) {
- hdr = blobmsg_hdr_from_blob(attr, len);
- if (!hdr)
+ if (!blobmsg_check_attr_len(attr, false, len))
return -1;
- if (!blobmsg_hdr_valid_namelen(hdr, len))
- return -1;
+ if (!blob_is_extended(attr))
+ continue;
+ hdr = blob_data(attr);
for (i = 0; i < policy_len; i++) {
if (!policy[i].name)
continue;
@@ -230,9 +207,6 @@ int blobmsg_parse(const struct blobmsg_p
if (blobmsg_namelen(hdr) != pslen[i])
continue;
- if (!blobmsg_check_attr_len(attr, true, len))
- return -1;
-
if (tb[i])
continue;