From 216159cfbf0b0a8c1c94e1030bd5f6846713f1ef Mon Sep 17 00:00:00 2001 From: starlet-dx <15929766099@163.com> Date: Fri, 1 Dec 2023 17:46:42 +0800 Subject: [PATCH] Fix CVE-2022-47630 --- CVE-2022-47630-1.patch | 50 +++++++++++++++++++++++ CVE-2022-47630-2.patch | 73 ++++++++++++++++++++++++++++++++++ CVE-2022-47630-3.patch | 84 +++++++++++++++++++++++++++++++++++++++ CVE-2022-47630-4.patch | 82 ++++++++++++++++++++++++++++++++++++++ arm-trusted-firmware.spec | 13 +++++- 5 files changed, 301 insertions(+), 1 deletion(-) create mode 100644 CVE-2022-47630-1.patch create mode 100644 CVE-2022-47630-2.patch create mode 100644 CVE-2022-47630-3.patch create mode 100644 CVE-2022-47630-4.patch diff --git a/CVE-2022-47630-1.patch b/CVE-2022-47630-1.patch new file mode 100644 index 0000000..1efce78 --- /dev/null +++ b/CVE-2022-47630-1.patch @@ -0,0 +1,50 @@ +From fd37982a19a4a2911912ce321b9468993a0919ad Mon Sep 17 00:00:00 2001 +From: Demi Marie Obenour +Date: Thu, 8 Dec 2022 15:23:56 -0500 +Subject: fix(auth): forbid junk after extensions + +The extensions must use all remaining bytes in the TBSCertificate. + +Change-Id: Idf48f7168e146d050ba62dbc732638946fcd6c92 +Signed-off-by: Demi Marie Obenour +--- + drivers/auth/mbedtls/mbedtls_x509_parser.c | 8 +++++--- + 1 file changed, 5 insertions(+), 3 deletions(-) + +diff --git a/drivers/auth/mbedtls/mbedtls_x509_parser.c b/drivers/auth/mbedtls/mbedtls_x509_parser.c +index 49bc008ed1..8c78003bb2 100644 +--- a/drivers/auth/mbedtls/mbedtls_x509_parser.c ++++ b/drivers/auth/mbedtls/mbedtls_x509_parser.c +@@ -304,24 +304,26 @@ static int cert_parse(void *img, unsigned int img_len) + + /* + * extensions [3] EXPLICIT Extensions OPTIONAL ++ * -- must use all remaining bytes in TBSCertificate + */ + ret = mbedtls_asn1_get_tag(&p, end, &len, + MBEDTLS_ASN1_CONTEXT_SPECIFIC | + MBEDTLS_ASN1_CONSTRUCTED | 3); +- if (ret != 0) { ++ if ((ret != 0) || (len != (size_t)(end - p))) { + return IMG_PARSER_ERR_FORMAT; + } + + /* + * Extensions ::= SEQUENCE SIZE (1..MAX) OF Extension ++ * -- must use all remaining bytes in TBSCertificate + */ + v3_ext.p = p; + ret = mbedtls_asn1_get_tag(&p, end, &len, MBEDTLS_ASN1_CONSTRUCTED | + MBEDTLS_ASN1_SEQUENCE); +- if (ret != 0) { ++ if ((ret != 0) || (len != (size_t)(end - p))) { + return IMG_PARSER_ERR_FORMAT; + } +- v3_ext.len = (p + len) - v3_ext.p; ++ v3_ext.len = end - v3_ext.p; + + /* + * Check extensions integrity +-- +cgit v1.2.3 + diff --git a/CVE-2022-47630-2.patch b/CVE-2022-47630-2.patch new file mode 100644 index 0000000..4a2a118 --- /dev/null +++ b/CVE-2022-47630-2.patch @@ -0,0 +1,73 @@ +From 72460f50e2437a85ce5229c430931aab8f4a0d5b Mon Sep 17 00:00:00 2001 +From: Demi Marie Obenour +Date: Thu, 8 Dec 2022 15:23:58 -0500 +Subject: fix(auth): require at least one extension to be present + +X.509 and RFC5280 allow omitting the extensions entirely, but require +that if the extensions field is present at all, it must contain at least +one certificate. TF-A already requires the extensions to be present, +but allows them to be empty. However, a certificate with an empty +extensions field will always fail later on, as the extensions contain +the information needed to validate the next stage in the boot chain. +Therefore, it is simpler to require the extension field to be present +and contain at least one extension. Also add a comment explaining why +the extensions field is required, even though it is OPTIONAL in the +ASN.1 syntax. + +Change-Id: Ie26eed8a7924bf50937a6b27ccdf7cc9a390588d +Signed-off-by: Demi Marie Obenour +--- + drivers/auth/mbedtls/mbedtls_x509_parser.c | 22 ++++++++++++++++++---- + 1 file changed, 18 insertions(+), 4 deletions(-) + +diff --git a/drivers/auth/mbedtls/mbedtls_x509_parser.c b/drivers/auth/mbedtls/mbedtls_x509_parser.c +index 8c78003bb2..9cccd964d4 100644 +--- a/drivers/auth/mbedtls/mbedtls_x509_parser.c ++++ b/drivers/auth/mbedtls/mbedtls_x509_parser.c +@@ -304,7 +304,18 @@ static int cert_parse(void *img, unsigned int img_len) + + /* + * extensions [3] EXPLICIT Extensions OPTIONAL +- * -- must use all remaining bytes in TBSCertificate ++ * } ++ * ++ * X.509 and RFC5280 allow omitting the extensions entirely. ++ * However, in TF-A, a certificate with no extensions would ++ * always fail later on, as the extensions contain the ++ * information needed to authenticate the next stage in the ++ * boot chain. Furthermore, get_ext() assumes that the ++ * extensions have been parsed into v3_ext, and allowing ++ * there to be no extensions would pointlessly complicate ++ * the code. Therefore, just reject certificates without ++ * extensions. This is also why version 1 and 2 certificates ++ * are rejected above. + */ + ret = mbedtls_asn1_get_tag(&p, end, &len, + MBEDTLS_ASN1_CONTEXT_SPECIFIC | +@@ -326,9 +337,12 @@ static int cert_parse(void *img, unsigned int img_len) + v3_ext.len = end - v3_ext.p; + + /* +- * Check extensions integrity ++ * Check extensions integrity. At least one extension is ++ * required: the ASN.1 specifies a minimum size of 1, and at ++ * least one extension is needed to authenticate the next stage ++ * in the boot chain. + */ +- while (p < end) { ++ do { + ret = mbedtls_asn1_get_tag(&p, end, &len, + MBEDTLS_ASN1_CONSTRUCTED | + MBEDTLS_ASN1_SEQUENCE); +@@ -356,7 +370,7 @@ static int cert_parse(void *img, unsigned int img_len) + return IMG_PARSER_ERR_FORMAT; + } + p += len; +- } ++ } while (p < end); + + if (p != end) { + return IMG_PARSER_ERR_FORMAT; +-- +cgit v1.2.3 + diff --git a/CVE-2022-47630-3.patch b/CVE-2022-47630-3.patch new file mode 100644 index 0000000..f79c23d --- /dev/null +++ b/CVE-2022-47630-3.patch @@ -0,0 +1,84 @@ +From f5c51855d36e399e6e22cc1eb94f6b58e51b3b6d Mon Sep 17 00:00:00 2001 +From: Demi Marie Obenour +Date: Fri, 9 Dec 2022 17:19:08 -0500 +Subject: fix(auth): properly validate X.509 extensions + +get_ext() does not check the return value of the various mbedtls_* +functions, as cert_parse() is assumed to have guaranteed that they will +always succeed. However, it passes the end of an extension as the end +pointer to these functions, whereas cert_parse() passes the end of the +TBSCertificate. Furthermore, cert_parse() does *not* check that the +contents of the extension have the same length as the extension itself. +Before fd37982a19a4a291 ("fix(auth): forbid junk after extensions"), +cert_parse() also does not check that the extension block extends to the +end of the TBSCertificate. + +This is a problem, as mbedtls_asn1_get_tag() leaves *p and *len +undefined on failure. In practice, this results in get_ext() continuing +to parse at different offsets than were used (and validated) by +cert_parse(), which means that the in-bounds guarantee provided by +cert_parse() no longer holds. + +This patch fixes the remaining flaw by enforcing that the contents of an +extension are the same length as the extension itself. + +Change-Id: Id4570f911402e34d5d6c799ae01a01f184c68d7c +Signed-off-by: Demi Marie Obenour +Signed-off-by: Sandrine Bailleux +--- + drivers/auth/mbedtls/mbedtls_x509_parser.c | 18 ++++++++++++------ + 1 file changed, 12 insertions(+), 6 deletions(-) + +diff --git a/drivers/auth/mbedtls/mbedtls_x509_parser.c b/drivers/auth/mbedtls/mbedtls_x509_parser.c +index 44b25ba72b..bef2f3d0a6 100644 +--- a/drivers/auth/mbedtls/mbedtls_x509_parser.c ++++ b/drivers/auth/mbedtls/mbedtls_x509_parser.c +@@ -355,33 +355,39 @@ static int cert_parse(void *img, unsigned int img_len) + * in the boot chain. + */ + do { ++ unsigned char *end_ext_data; ++ + ret = mbedtls_asn1_get_tag(&p, end, &len, + MBEDTLS_ASN1_CONSTRUCTED | + MBEDTLS_ASN1_SEQUENCE); + if (ret != 0) { + return IMG_PARSER_ERR_FORMAT; + } ++ end_ext_data = p + len; + + /* Get extension ID */ +- ret = mbedtls_asn1_get_tag(&p, end, &len, MBEDTLS_ASN1_OID); ++ ret = mbedtls_asn1_get_tag(&p, end_ext_data, &len, MBEDTLS_ASN1_OID); + if (ret != 0) { + return IMG_PARSER_ERR_FORMAT; + } + p += len; + + /* Get optional critical */ +- ret = mbedtls_asn1_get_bool(&p, end, &is_critical); ++ ret = mbedtls_asn1_get_bool(&p, end_ext_data, &is_critical); + if ((ret != 0) && (ret != MBEDTLS_ERR_ASN1_UNEXPECTED_TAG)) { + return IMG_PARSER_ERR_FORMAT; + } + +- /* Data should be octet string type */ +- ret = mbedtls_asn1_get_tag(&p, end, &len, ++ /* ++ * Data should be octet string type and must use all bytes in ++ * the Extension. ++ */ ++ ret = mbedtls_asn1_get_tag(&p, end_ext_data, &len, + MBEDTLS_ASN1_OCTET_STRING); +- if (ret != 0) { ++ if ((ret != 0) || ((p + len) != end_ext_data)) { + return IMG_PARSER_ERR_FORMAT; + } +- p += len; ++ p = end_ext_data; + } while (p < end); + + if (p != end) { +-- +cgit v1.2.3 + diff --git a/CVE-2022-47630-4.patch b/CVE-2022-47630-4.patch new file mode 100644 index 0000000..cf427a1 --- /dev/null +++ b/CVE-2022-47630-4.patch @@ -0,0 +1,82 @@ +From abb8f936fd0ad085b1966bdc2cddf040ba3865e3 Mon Sep 17 00:00:00 2001 +From: Demi Marie Obenour +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 +--- + 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 + diff --git a/arm-trusted-firmware.spec b/arm-trusted-firmware.spec index 05dc107..02eb33c 100644 --- a/arm-trusted-firmware.spec +++ b/arm-trusted-firmware.spec @@ -2,7 +2,7 @@ Name: arm-trusted-firmware Version: 1.6 -Release: 2 +Release: 3 Summary: ARM Trusted Firmware License: BSD URL: https://github.com/ARM-software/arm-trusted-firmware/wiki @@ -11,6 +11,14 @@ Patch0000: AArch32-Disable-Secure-Cycle-Counter.patch Patch0001: cleanup-context--handing-library.patch Patch0002: use-helper-function-to-save-registers-in-SMC-handler.patch Patch0003: AArch64-Disable-Secure-Cycle-Counter.patch +# https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/commit/?id=fd37982a19a4a291 +Patch0004: CVE-2022-47630-1.patch +# https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/commit/?id=72460f50e2437a85 +Patch0005: CVE-2022-47630-2.patch +# https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/commit/?id=f5c51855d36e399e +Patch0006: CVE-2022-47630-3.patch +# https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/commit/?id=abb8f936fd0ad085 +Patch0007: CVE-2022-47630-4.patch ExclusiveArch: aarch64 BuildRequires: dtc @@ -62,6 +70,9 @@ done %{_datadir}/%{name} %changelog +* Fri Dec 01 2023 yaoxin - 1.6-3 +- Fix CVE-2022-47630 + * Wed Sep 16 2020 wangyue - 1.6-2 - fix CVE-2017-15031 -- Gitee