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 <mschiffer@universe-factory.net>
struct blob_attr *certtb[CERT_ATTR_MAX];
struct blob_attr *bufpt;
struct cert_object *cobj;
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);
ssize_t len;
len = read_file(certfile, filebuf, sizeof(filebuf) - 1, 0);
}
bufpt = (struct blob_attr *)filebuf;
}
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)
- 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 */
- else
- pos += blob_pad_len(bufpt);
if (!certtb[CERT_ATTR_SIGNATURE])
/* no signature -> drop */
if (!certtb[CERT_ATTR_SIGNATURE])
/* no signature -> drop */
cobj->cert[CERT_ATTR_PAYLOAD] = blob_memdup(certtb[CERT_ATTR_PAYLOAD]);
list_add_tail(&cobj->list, 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);
+ }