Skip to content

Commit e64a26a

Browse files
sandrine-bailleux-armTrustedFirmware Code Review
authored andcommitted
Merge "docs(security): security advisory for CVE-2022-47630" into integration
2 parents f53a1b6 + d7156d4 commit e64a26a

File tree

2 files changed

+160
-0
lines changed

2 files changed

+160
-0
lines changed

docs/security_advisories/index.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,4 @@ Security Advisories
1414
security-advisory-tfv-7.rst
1515
security-advisory-tfv-8.rst
1616
security-advisory-tfv-9.rst
17+
security-advisory-tfv-10.rst
Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
Advisory TFV-10 (CVE-2022-47630)
2+
================================
3+
4+
+----------------+-------------------------------------------------------------+
5+
| Title | Incorrect validation of X.509 certificate extensions can |
6+
| | result in an out-of-bounds read. |
7+
+================+=============================================================+
8+
| CVE ID | `CVE-2022-47630`_ |
9+
+----------------+-------------------------------------------------------------+
10+
| Date | Reported on 12 Dec 2022 |
11+
+----------------+-------------------------------------------------------------+
12+
| Versions | v1.2 to v2.8 |
13+
| Affected | |
14+
+----------------+-------------------------------------------------------------+
15+
| Configurations | BL1 and BL2 with Trusted Boot enabled with custom, |
16+
| Affected | downstream usages of ``get_ext()`` and/or ``auth_nvctr()`` |
17+
| | interfaces. Not exploitable in upstream TF-A code. |
18+
+----------------+-------------------------------------------------------------+
19+
| Impact | Out-of-bounds read. |
20+
+----------------+-------------------------------------------------------------+
21+
| Fix Version | - `fd37982a19a4a291`_ "fix(auth): forbid junk after |
22+
| | extensions" |
23+
| | |
24+
| | - `72460f50e2437a85`_ "fix(auth): require at least one |
25+
| | extension to be present" |
26+
| | |
27+
| | - `f5c51855d36e399e`_ "fix(auth): properly validate X.509 |
28+
| | extensions" |
29+
| | |
30+
| | - `abb8f936fd0ad085`_ "fix(auth): avoid out-of-bounds read |
31+
| | in auth_nvctr()" |
32+
| | |
33+
| | Note that `72460f50e2437a85`_ is not fixing any |
34+
| | vulnerability per se but it is required for |
35+
| | `f5c51855d36e399e`_ to apply cleanly. |
36+
+----------------+-------------------------------------------------------------+
37+
| Credit | Demi Marie Obenour, Invisible Things Lab |
38+
+----------------+-------------------------------------------------------------+
39+
40+
This security advisory describes a vulnerability in the X.509 parser used to
41+
parse boot certificates in TF-A trusted boot: it is possible for a crafted
42+
certificate to cause an out-of-bounds memory read.
43+
44+
Note that upstream platforms are **not** affected by this. Only downstream
45+
platforms may be, if (and only if) the interfaces described below are used in a
46+
different context than seen in upstream code. Details of such context is
47+
described in the rest of this document.
48+
49+
To fully understand this security advisory, it is recommended to refer to the
50+
following standards documents:
51+
52+
- `RFC 5280`_, *Internet X.509 Public Key Infrastructure Certificate and
53+
Certificate Revocation List (CRL) Profile*.
54+
55+
- `ITU-T X.690`_, *ASN.1 encoding rules: Specification of Basic Encoding Rules
56+
(BER), Canonical Encoding Rules (CER) and Distinguished Encoding Rules
57+
(DER).*
58+
59+
Bug 1: Insufficient certificate validation
60+
------------------------------------------
61+
62+
The vulnerability lies in the following source file:
63+
``drivers/auth/mbedtls/mbedtls_x509_parser.c``. By design, ``get_ext()`` does
64+
not check the return value of the various ``mbedtls_*()`` functions, as
65+
``cert_parse()`` is assumed to have guaranteed that they will always succeed.
66+
However, it passes the end of an extension as the end pointer to these
67+
functions, whereas ``cert_parse()`` passes the end of the ``TBSCertificate``.
68+
Furthermore, ``cert_parse()`` does not check that the contents of the extension
69+
have the same length as the extension itself. It also does not check that the
70+
extension block extends to the end of the ``TBSCertificate``.
71+
72+
This is a problem, as ``mbedtls_asn1_get_tag()`` leaves ``*p`` and ``*len``
73+
undefined on failure. In practice, this results in ``get_ext()`` continuing to
74+
parse at different offsets than were used (and validated) by ``cert_parse()``,
75+
which means that the in-bounds guarantee provided by ``cert_parse()`` no longer
76+
holds. The result is that it is possible for ``get_ext()`` to read memory past
77+
the end of the certificate. This could potentially access memory with dangerous
78+
read side effects, or leak microarchitectural state that could theoretically be
79+
retrieved through some side-channel attacks as part of a more complex attack.
80+
81+
Bug 2: Missing bounds check in ``auth_nvctr()``
82+
-----------------------------------------------
83+
``auth_nvctr()`` does not check that the buffer provided is
84+
long enough to hold an ``ASN.1 INTEGER``. Since ``auth_nvctr()`` will only ever
85+
read 6 bytes, it is possible to read up to 6 bytes past the end of the buffer.
86+
87+
Exploitability Analysis
88+
-----------------------
89+
90+
Upstream TF-A Code
91+
~~~~~~~~~~~~~~~~~~
92+
93+
In upstream TF-A code, the only caller of ``auth_nvctr()`` takes its input from
94+
``get_ext()``, which means that the second bug is exploitable, so is the first.
95+
Therefore, only the first bug need be considered.
96+
97+
All standard chains of trust provided in TF-A source tree (that is, under
98+
``drivers/auth/``) require that the certificate's signature has already been
99+
validated prior to calling ``get_ext()``, or any function that calls ``get_ext()``.
100+
Platforms taking their chain of trust from a dynamic configuration file (such as
101+
``fdts/cot_descriptors.dtsi``) are also safe, as signature verification will
102+
always be done prior to any calls to ``get_ext()`` or ``auth_nvctr()`` in this
103+
case, no matter the order of the properties in the file. Therefore, it is not
104+
possible to exploit this vulnerability pre-authentication in upstream TF-A.
105+
106+
Furthermore, the data read through ``get_ext()`` only
107+
ever gets used by the authentication framework (``drivers/auth/auth_mod.c``),
108+
which greatly reduces the range of inputs it will ever receive and thus the
109+
impact this has. Specifically, the authentication framework uses ``get_ext()``
110+
in three cases:
111+
112+
1. Retrieving a hash from an X.509 certificate to check the integrity of a
113+
child certificate (see ``auth_hash()``).
114+
115+
2. Retrieving the signature details from an X.509 certificate to check its
116+
authenticity and integrity (see ``auth_signature()``).
117+
118+
3. Retrieving the security counter value from an X.509 certificate to protect
119+
it from unauthorized rollback to a previous version (see ``auth_nvctr()``).
120+
121+
None of these uses authentication framework write to the out-of-bounds memory,
122+
so no memory corruption is possible.
123+
124+
In summary, there are 2 separate issues - one in ``get_ext()`` and another one
125+
in ``auth_nvctr()`` - but neither of these can be exploited in the context of
126+
TF-A upstream code.
127+
128+
Only in the following 2 cases do we expect this vulnerability to be triggerable
129+
prior to authentication:
130+
131+
- The platform uses a custom chain of trust which uses the non-volatile counter
132+
authentication method (``AUTH_METHOD_NV_CTR``) before the cryptographic
133+
authentication method (``AUTH_METHOD_SIG``).
134+
135+
- The chain of trust uses a custom authentication method that calls
136+
``get_ext()`` before cryptographic authentication.
137+
138+
Custom Image Parsers
139+
~~~~~~~~~~~~~~~~~~~~
140+
141+
If the platform uses a custom image parser instead of the certificate parser,
142+
the bug in the certificate parser is obviously not relevant. The bug in
143+
``auth_nvctr()`` *may* be relevant, but only if the returned data is:
144+
145+
- Taken from an untrusted source (meaning that it is read prior to
146+
authentication).
147+
148+
- Not already checked to be a primitively-encoded ASN.1 tag.
149+
150+
In particular, if the custom image parser implementation wraps a 32-bit integer
151+
in an ASN.1 ``INTEGER``, it is not affected.
152+
153+
.. _CVE-2022-47630: http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-47630
154+
.. _fd37982a19a4a291: https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/commit/?id=fd37982a19a4a291
155+
.. _72460f50e2437a85: https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/commit/?id=72460f50e2437a85
156+
.. _f5c51855d36e399e: https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/commit/?id=f5c51855d36e399e
157+
.. _abb8f936fd0ad085: https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/commit/?id=abb8f936fd0ad085
158+
.. _RFC 5280: https://www.ietf.org/rfc/rfc5280.txt
159+
.. _ITU-T X.690: https://www.itu.int/ITU-T/studygroups/com10/languages/X.690_1297.pdf

0 commit comments

Comments
 (0)