cert_load() iterates over multiple blobs, so the length argument to
blob_parse_untrusted() needs to be updated to prevent out-of-bounds
accesses.
Some other checks have become redundant and are removed, as
blob_parse_untrusted() already ensures that all attrs are contained in
the passed buffer.
Note that this issue currently does not pose a security threat, as an
over-restrictive check in blob_parse_untrusted() broke parsing of
buffers with multiple blobs completely.
Signed-off-by: Matthias Schiffer <mschif...@universe-factory.net>
---
ucert.c | 34 +++++++++++++++++++---------------
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/ucert.c b/ucert.c
index 208d5f67e10d..24349c41349b 100644
--- a/ucert.c
+++ b/ucert.c
@@ -164,9 +164,8 @@ static int cert_load(const char *certfile, struct list_head
*chain) {
struct blob_attr *certtb[CERT_ATTR_MAX];
struct blob_attr *bufpt;
struct cert_object *cobj;
- char filebuf[CERT_BUF_LEN];
- int ret = 0, pret = 0;
- size_t pos = 0;
+ char filebuf[CERT_BUF_LEN], *end;
+ int ret = 1;
ssize_t len;
len = read_file(certfile, filebuf, sizeof(filebuf) - 1, 0);
@@ -177,17 +176,16 @@ static int cert_load(const char *certfile, struct
list_head *chain) {
}
bufpt = (struct blob_attr *)filebuf;
- do {
- pret = blob_parse_untrusted(bufpt, len, certtb, cert_policy,
CERT_ATTR_MAX);
- if (pret <= 0)
- /* no attributes found */
+ end = filebuf + len;
+
+ while (true) {
+ len = end - (char *)bufpt;
+ if (len <= 0)
break;
- if (pos + blob_pad_len(bufpt) > (size_t) len)
- /* blob exceeds filebuffer */
+ if (blob_parse_untrusted(bufpt, len, certtb, cert_policy,
CERT_ATTR_MAX) <= 0)
+ /* no attributes found */
break;
- else
- pos += blob_pad_len(bufpt);
if (!certtb[CERT_ATTR_SIGNATURE])
/* no signature -> drop */
@@ -199,11 +197,17 @@ static int cert_load(const char *certfile, struct
list_head *chain) {
cobj->cert[CERT_ATTR_PAYLOAD] =
blob_memdup(certtb[CERT_ATTR_PAYLOAD]);
list_add_tail(&cobj->list, chain);
- ret += pret;
- /* repeat parsing while there is still enough remaining data in buffer
*/
- } while((size_t) len > pos + sizeof(struct blob_attr) && (bufpt =
blob_next(bufpt)));
+ ret = 0;
+
+ /* Repeat parsing while there is still enough remaining data in
buffer
+ *
+ * Note that blob_next() is only valid for untrusted data
because blob_parse_untrusted()
+ * verified that the buffer contains at least one blob, and
that it is completely contained
+ * in the buffer */
+ bufpt = blob_next(bufpt);
+ }
- return (ret <= 0);
+ return ret;
}
#ifdef UCERT_FULL
--
2.26.2
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
