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