83 lines
3.0 KiB
Diff
83 lines
3.0 KiB
Diff
|
From abb8f936fd0ad085b1966bdc2cddf040ba3865e3 Mon Sep 17 00:00:00 2001
|
|||
|
|
From: Demi Marie Obenour <demiobenour@gmail.com>
|
|||
|
|
Date: Fri, 9 Dec 2022 18:21:47 -0500
|
|||
|
|
Subject: fix(auth): avoid out-of-bounds read in auth_nvctr()
|
|||
|
|
MIME-Version: 1.0
|
|||
|
|
Content-Type: text/plain; charset=UTF-8
|
|||
|
|
Content-Transfer-Encoding: 8bit
|
|||
|
|
|
|||
|
|
auth_nvctr() does not check that the buffer provided is long enough to
|
|||
|
|
hold an ASN.1 INTEGER, or even that the buffer is non-empty. Since
|
|||
|
|
auth_nvctr() will only ever read 6 bytes, it is possible to read up to
|
|||
|
|
6 bytes past the end of the buffer.
|
|||
|
|
|
|||
|
|
This out-of-bounds read turns out to be harmless. The only caller of
|
|||
|
|
auth_nvctr() always passes a pointer into an X.509 TBSCertificate, and
|
|||
|
|
all in-tree chains of trust require that the certificate’s signature has
|
|||
|
|
already been validated. This means that the signature algorithm
|
|||
|
|
identifier is at least 4 bytes and the signature itself more than that.
|
|||
|
|
Therefore, the data read will be from the certificate itself. Even if
|
|||
|
|
the certificate signature has not been validated, an out-of-bounds read
|
|||
|
|
is still not possible. Since there are at least two bytes (tag and
|
|||
|
|
length) in both the signature algorithm ID and the signature itself, an
|
|||
|
|
out-of-bounds read would require that the tag byte of the signature
|
|||
|
|
algorithm ID would need to be either the tag or length byte of the
|
|||
|
|
DER-encoded nonvolatile counter. However, this byte must be
|
|||
|
|
(MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE) (0x30), which is
|
|||
|
|
greater than 4 and not equal to MBEDTLS_ASN1_INTEGER (2). Therefore,
|
|||
|
|
auth_nvctr() will error out before reading the integer itself,
|
|||
|
|
preventing an out-of-bounds read.
|
|||
|
|
|
|||
|
|
Change-Id: Ibdf1af702fbeb98a94c0c96456ebddd3d392ad44
|
|||
|
|
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
|
|||
|
|
---
|
|||
|
|
drivers/auth/auth_mod.c | 20 ++++++++++++++------
|
|||
|
|
1 file changed, 14 insertions(+), 6 deletions(-)
|
|||
|
|
|
|||
|
|
diff --git a/drivers/auth/auth_mod.c b/drivers/auth/auth_mod.c
|
|||
|
|
index eb537b6..070f60f 100644
|
|||
|
|
--- a/drivers/auth/auth_mod.c
|
|||
|
|
+++ b/drivers/auth/auth_mod.c
|
|||
|
|
@@ -228,7 +228,7 @@ static int auth_nvctr(const auth_method_param_nv_ctr_t *param,
|
|||
|
|
const auth_img_desc_t *img_desc,
|
|||
|
|
void *img, unsigned int img_len)
|
|||
|
|
{
|
|||
|
|
- char *p;
|
|||
|
|
+ unsigned char *p;
|
|||
|
|
void *data_ptr = NULL;
|
|||
|
|
unsigned int data_len, len, i;
|
|||
|
|
unsigned int cert_nv_ctr, plat_nv_ctr;
|
|||
|
|
@@ -242,16 +242,24 @@ static int auth_nvctr(const auth_method_param_nv_ctr_t *param,
|
|||
|
|
|
|||
|
|
/* Parse the DER encoded integer */
|
|||
|
|
assert(data_ptr);
|
|||
|
|
- p = (char *)data_ptr;
|
|||
|
|
- if (*p != ASN1_INTEGER) {
|
|||
|
|
+ p = (unsigned char *)data_ptr;
|
|||
|
|
+
|
|||
|
|
+ /*
|
|||
|
|
+ * Integers must be at least 3 bytes: 1 for tag, 1 for length, and 1
|
|||
|
|
+ * for value. The first byte (tag) must be ASN1_INTEGER.
|
|||
|
|
+ */
|
|||
|
|
+ if ((data_len < 3) || (*p != ASN1_INTEGER)) {
|
|||
|
|
/* Invalid ASN.1 integer */
|
|||
|
|
return 1;
|
|||
|
|
}
|
|||
|
|
p++;
|
|||
|
|
|
|||
|
|
- /* NV-counters are unsigned integers up to 32-bit */
|
|||
|
|
- len = (unsigned int)(*p & 0x7f);
|
|||
|
|
- if ((*p & 0x80) || (len > 4)) {
|
|||
|
|
+ /*
|
|||
|
|
+ * NV-counters are unsigned integers up to 31 bits. Trailing
|
|||
|
|
+ * padding is not allowed.
|
|||
|
|
+ */
|
|||
|
|
+ len = (unsigned int)*p;
|
|||
|
|
+ if ((len > 4) || (data_len - 2 != len)) {
|
|||
|
|
return 1;
|
|||
|
|
}
|
|||
|
|
p++;
|
|||
|
|
--
|
|||
|
|
2.30.0
|
|||
|
|
|