xref: /arm-trusted-firmware/docs/security_advisories/security-advisory-tfv-1.rst (revision 91f16700b400a8c0651d24a598fc48ee2997a0d7)
1*91f16700SchasingluluAdvisory TFV-1 (CVE-2016-10319)
2*91f16700Schasinglulu===============================
3*91f16700Schasinglulu
4*91f16700Schasinglulu+----------------+-------------------------------------------------------------+
5*91f16700Schasinglulu| Title          | Malformed Firmware Update SMC can result in copy of         |
6*91f16700Schasinglulu|                | unexpectedly large data into secure memory                  |
7*91f16700Schasinglulu+================+=============================================================+
8*91f16700Schasinglulu| CVE ID         | `CVE-2016-10319`_                                           |
9*91f16700Schasinglulu+----------------+-------------------------------------------------------------+
10*91f16700Schasinglulu| Date           | 18 Oct 2016                                                 |
11*91f16700Schasinglulu+----------------+-------------------------------------------------------------+
12*91f16700Schasinglulu| Versions       | v1.2 and v1.3 (since commit `48bfb88`_)                     |
13*91f16700Schasinglulu| Affected       |                                                             |
14*91f16700Schasinglulu+----------------+-------------------------------------------------------------+
15*91f16700Schasinglulu| Configurations | Platforms that use AArch64 BL1 plus untrusted normal world  |
16*91f16700Schasinglulu| Affected       | firmware update code executing before BL31                  |
17*91f16700Schasinglulu+----------------+-------------------------------------------------------------+
18*91f16700Schasinglulu| Impact         | Copy of unexpectedly large data into the free secure memory |
19*91f16700Schasinglulu|                | reported by BL1 platform code                               |
20*91f16700Schasinglulu+----------------+-------------------------------------------------------------+
21*91f16700Schasinglulu| Fix Version    | `Pull Request #783`_                                        |
22*91f16700Schasinglulu+----------------+-------------------------------------------------------------+
23*91f16700Schasinglulu| Credit         | IOActive                                                    |
24*91f16700Schasinglulu+----------------+-------------------------------------------------------------+
25*91f16700Schasinglulu
26*91f16700SchasingluluGeneric Trusted Firmware (TF) BL1 code contains an SMC interface that is briefly
27*91f16700Schasingluluavailable after cold reset to support the Firmware Update (FWU) feature (also
28*91f16700Schasingluluknown as recovery mode). This allows most FWU functionality to be implemented in
29*91f16700Schasingluluthe normal world, while retaining the essential image authentication
30*91f16700Schasinglulufunctionality in BL1. When cold boot reaches the EL3 Runtime Software (for
31*91f16700Schasingluluexample, BL31 on AArch64 systems), the FWU SMC interface is replaced by the EL3
32*91f16700SchasingluluRuntime SMC interface. Platforms may choose how much of this FWU functionality
33*91f16700Schasingluluto use, if any.
34*91f16700Schasinglulu
35*91f16700SchasingluluThe BL1 FWU SMC handling code, currently only supported on AArch64, contains
36*91f16700Schasingluluseveral vulnerabilities that may be exploited when *all* the following
37*91f16700Schasingluluconditions apply:
38*91f16700Schasinglulu
39*91f16700Schasinglulu1. Platform code uses TF BL1 with the ``TRUSTED_BOARD_BOOT`` build option
40*91f16700Schasinglulu   enabled.
41*91f16700Schasinglulu
42*91f16700Schasinglulu2. Platform code arranges for untrusted normal world FWU code to be executed in
43*91f16700Schasinglulu   the cold boot path, before BL31 starts. Untrusted in this sense means code
44*91f16700Schasinglulu   that is not in ROM or has not been authenticated or has otherwise been
45*91f16700Schasinglulu   executed by an attacker.
46*91f16700Schasinglulu
47*91f16700Schasinglulu3. Platform code copies the insecure pattern described below from the ARM
48*91f16700Schasinglulu   platform version of ``bl1_plat_mem_check()``.
49*91f16700Schasinglulu
50*91f16700SchasingluluThe vulnerabilities consist of potential integer overflows in the input
51*91f16700Schasingluluvalidation checks while handling the ``FWU_SMC_IMAGE_COPY`` SMC. The SMC
52*91f16700Schasingluluimplementation is designed to copy an image into secure memory for subsequent
53*91f16700Schasingluluauthentication, but the vulnerabilities may allow an attacker to copy
54*91f16700Schasingluluunexpectedly large data into secure memory. Note that a separate vulnerability
55*91f16700Schasingluluis required to leverage these vulnerabilities; for example a way to get the
56*91f16700Schasinglulusystem to change its behaviour based on the unexpected secure memory contents.
57*91f16700Schasinglulu
58*91f16700SchasingluluTwo of the vulnerabilities are in the function ``bl1_fwu_image_copy()`` in
59*91f16700Schasinglulu``bl1/bl1_fwu.c``. These are listed below, referring to the v1.3 tagged version
60*91f16700Schasingluluof the code:
61*91f16700Schasinglulu
62*91f16700Schasinglulu- Line 155:
63*91f16700Schasinglulu
64*91f16700Schasinglulu  .. code:: c
65*91f16700Schasinglulu
66*91f16700Schasinglulu    /*
67*91f16700Schasinglulu     * If last block is more than expected then
68*91f16700Schasinglulu     * clip the block to the required image size.
69*91f16700Schasinglulu     */
70*91f16700Schasinglulu    if (image_desc->copied_size + block_size >
71*91f16700Schasinglulu         image_desc->image_info.image_size) {
72*91f16700Schasinglulu        block_size = image_desc->image_info.image_size -
73*91f16700Schasinglulu            image_desc->copied_size;
74*91f16700Schasinglulu        WARN("BL1-FWU: Copy argument block_size > remaining image size."
75*91f16700Schasinglulu            " Clipping block_size\n");
76*91f16700Schasinglulu    }
77*91f16700Schasinglulu
78*91f16700Schasinglulu    /* Make sure the image src/size is mapped. */
79*91f16700Schasinglulu    if (bl1_plat_mem_check(image_src, block_size, flags)) {
80*91f16700Schasinglulu        WARN("BL1-FWU: Copy arguments source/size not mapped\n");
81*91f16700Schasinglulu        return -ENOMEM;
82*91f16700Schasinglulu    }
83*91f16700Schasinglulu
84*91f16700Schasinglulu    INFO("BL1-FWU: Continuing image copy in blocks\n");
85*91f16700Schasinglulu
86*91f16700Schasinglulu    /* Copy image for given block size. */
87*91f16700Schasinglulu    base_addr += image_desc->copied_size;
88*91f16700Schasinglulu    image_desc->copied_size += block_size;
89*91f16700Schasinglulu    memcpy((void *)base_addr, (const void *)image_src, block_size);
90*91f16700Schasinglulu    ...
91*91f16700Schasinglulu
92*91f16700Schasinglulu  This code fragment is executed when the image copy operation is performed in
93*91f16700Schasinglulu  blocks over multiple SMCs. ``block_size`` is an SMC argument and therefore
94*91f16700Schasinglulu  potentially controllable by an attacker. A very large value may result in an
95*91f16700Schasinglulu  integer overflow in the 1st ``if`` statement, which would bypass the check,
96*91f16700Schasinglulu  allowing an unclipped ``block_size`` to be passed into
97*91f16700Schasinglulu  ``bl1_plat_mem_check()``. If ``bl1_plat_mem_check()`` also passes, this may
98*91f16700Schasinglulu  result in an unexpectedly large copy of data into secure memory.
99*91f16700Schasinglulu
100*91f16700Schasinglulu- Line 206:
101*91f16700Schasinglulu
102*91f16700Schasinglulu  .. code:: c
103*91f16700Schasinglulu
104*91f16700Schasinglulu    /* Make sure the image src/size is mapped. */
105*91f16700Schasinglulu    if (bl1_plat_mem_check(image_src, block_size, flags)) {
106*91f16700Schasinglulu        WARN("BL1-FWU: Copy arguments source/size not mapped\n");
107*91f16700Schasinglulu        return -ENOMEM;
108*91f16700Schasinglulu    }
109*91f16700Schasinglulu
110*91f16700Schasinglulu    /* Find out how much free trusted ram remains after BL1 load */
111*91f16700Schasinglulu    mem_layout = bl1_plat_sec_mem_layout();
112*91f16700Schasinglulu    if ((image_desc->image_info.image_base < mem_layout->free_base) ||
113*91f16700Schasinglulu         (image_desc->image_info.image_base + image_size >
114*91f16700Schasinglulu          mem_layout->free_base + mem_layout->free_size)) {
115*91f16700Schasinglulu        WARN("BL1-FWU: Memory not available to copy\n");
116*91f16700Schasinglulu        return -ENOMEM;
117*91f16700Schasinglulu    }
118*91f16700Schasinglulu
119*91f16700Schasinglulu    /* Update the image size. */
120*91f16700Schasinglulu    image_desc->image_info.image_size = image_size;
121*91f16700Schasinglulu
122*91f16700Schasinglulu    /* Copy image for given size. */
123*91f16700Schasinglulu    memcpy((void *)base_addr, (const void *)image_src, block_size);
124*91f16700Schasinglulu    ...
125*91f16700Schasinglulu
126*91f16700Schasinglulu  This code fragment is executed during the 1st invocation of the image copy
127*91f16700Schasinglulu  operation. Both ``block_size`` and ``image_size`` are SMC arguments. A very
128*91f16700Schasinglulu  large value of ``image_size`` may result in an integer overflow in the 2nd
129*91f16700Schasinglulu  ``if`` statement, which would bypass the check, allowing execution to proceed.
130*91f16700Schasinglulu  If ``bl1_plat_mem_check()`` also passes, this may result in an unexpectedly
131*91f16700Schasinglulu  large copy of data into secure memory.
132*91f16700Schasinglulu
133*91f16700SchasingluluIf the platform's implementation of ``bl1_plat_mem_check()`` is correct then it
134*91f16700Schasinglulumay help prevent the above 2 vulnerabilities from being exploited. However, the
135*91f16700SchasingluluARM platform version of this function contains a similar vulnerability:
136*91f16700Schasinglulu
137*91f16700Schasinglulu- Line 88 of ``plat/arm/common/arm_bl1_fwu.c`` in function of
138*91f16700Schasinglulu  ``bl1_plat_mem_check()``:
139*91f16700Schasinglulu
140*91f16700Schasinglulu  .. code:: c
141*91f16700Schasinglulu
142*91f16700Schasinglulu    while (mmap[index].mem_size) {
143*91f16700Schasinglulu        if ((mem_base >= mmap[index].mem_base) &&
144*91f16700Schasinglulu            ((mem_base + mem_size)
145*91f16700Schasinglulu            <= (mmap[index].mem_base +
146*91f16700Schasinglulu            mmap[index].mem_size)))
147*91f16700Schasinglulu            return 0;
148*91f16700Schasinglulu
149*91f16700Schasinglulu        index++;
150*91f16700Schasinglulu    }
151*91f16700Schasinglulu    ...
152*91f16700Schasinglulu
153*91f16700Schasinglulu  This function checks that the passed memory region is within one of the
154*91f16700Schasinglulu  regions mapped in by ARM platforms. Here, ``mem_size`` may be the
155*91f16700Schasinglulu  ``block_size`` passed from ``bl1_fwu_image_copy()``. A very large value of
156*91f16700Schasinglulu  ``mem_size`` may result in an integer overflow and the function to incorrectly
157*91f16700Schasinglulu  return success. Platforms that copy this insecure pattern will have the same
158*91f16700Schasinglulu  vulnerability.
159*91f16700Schasinglulu
160*91f16700Schasinglulu.. _CVE-2016-10319: http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-10319
161*91f16700Schasinglulu.. _48bfb88: https://github.com/ARM-software/arm-trusted-firmware/commit/48bfb88
162*91f16700Schasinglulu.. _Pull Request #783: https://github.com/ARM-software/arm-trusted-firmware/pull/783
163