xref: /rk3399_ARM-atf/docs/security_advisories/security-advisory-tfv-1.rst (revision ced1711297347f24fee45e75e73c7767507a0982)
1*267f8085SPaul BeesleyAdvisory TFV-1 (CVE-2016-10319)
2*267f8085SPaul Beesley===============================
3*267f8085SPaul Beesley
44fe91230SJoel Hutton+----------------+-------------------------------------------------------------+
54fe91230SJoel Hutton| Title          | Malformed Firmware Update SMC can result in copy of         |
64fe91230SJoel Hutton|                | unexpectedly large data into secure memory                  |
74fe91230SJoel Hutton+================+=============================================================+
812fc6ba7SPaul Beesley| CVE ID         | `CVE-2016-10319`_                                           |
94fe91230SJoel Hutton+----------------+-------------------------------------------------------------+
104fe91230SJoel Hutton| Date           | 18 Oct 2016                                                 |
114fe91230SJoel Hutton+----------------+-------------------------------------------------------------+
124fe91230SJoel Hutton| Versions       | v1.2 and v1.3 (since commit `48bfb88`_)                     |
134fe91230SJoel Hutton| Affected       |                                                             |
144fe91230SJoel Hutton+----------------+-------------------------------------------------------------+
154fe91230SJoel Hutton| Configurations | Platforms that use AArch64 BL1 plus untrusted normal world  |
164fe91230SJoel Hutton| Affected       | firmware update code executing before BL31                  |
174fe91230SJoel Hutton+----------------+-------------------------------------------------------------+
184fe91230SJoel Hutton| Impact         | Copy of unexpectedly large data into the free secure memory |
194fe91230SJoel Hutton|                | reported by BL1 platform code                               |
204fe91230SJoel Hutton+----------------+-------------------------------------------------------------+
214fe91230SJoel Hutton| Fix Version    | `Pull Request #783`_                                        |
224fe91230SJoel Hutton+----------------+-------------------------------------------------------------+
234fe91230SJoel Hutton| Credit         | IOActive                                                    |
244fe91230SJoel Hutton+----------------+-------------------------------------------------------------+
254fe91230SJoel Hutton
264fe91230SJoel HuttonGeneric Trusted Firmware (TF) BL1 code contains an SMC interface that is briefly
274fe91230SJoel Huttonavailable after cold reset to support the Firmware Update (FWU) feature (also
284fe91230SJoel Huttonknown as recovery mode). This allows most FWU functionality to be implemented in
294fe91230SJoel Huttonthe normal world, while retaining the essential image authentication
304fe91230SJoel Huttonfunctionality in BL1. When cold boot reaches the EL3 Runtime Software (for
314fe91230SJoel Huttonexample, BL31 on AArch64 systems), the FWU SMC interface is replaced by the EL3
324fe91230SJoel HuttonRuntime SMC interface. Platforms may choose how much of this FWU functionality
334fe91230SJoel Huttonto use, if any.
344fe91230SJoel Hutton
354fe91230SJoel HuttonThe BL1 FWU SMC handling code, currently only supported on AArch64, contains
364fe91230SJoel Huttonseveral vulnerabilities that may be exploited when *all* the following
374fe91230SJoel Huttonconditions apply:
384fe91230SJoel Hutton
394fe91230SJoel Hutton1. Platform code uses TF BL1 with the ``TRUSTED_BOARD_BOOT`` build option
404fe91230SJoel Hutton   enabled.
414fe91230SJoel Hutton
424fe91230SJoel Hutton2. Platform code arranges for untrusted normal world FWU code to be executed in
434fe91230SJoel Hutton   the cold boot path, before BL31 starts. Untrusted in this sense means code
444fe91230SJoel Hutton   that is not in ROM or has not been authenticated or has otherwise been
454fe91230SJoel Hutton   executed by an attacker.
464fe91230SJoel Hutton
474fe91230SJoel Hutton3. Platform code copies the insecure pattern described below from the ARM
484fe91230SJoel Hutton   platform version of ``bl1_plat_mem_check()``.
494fe91230SJoel Hutton
504fe91230SJoel HuttonThe vulnerabilities consist of potential integer overflows in the input
514fe91230SJoel Huttonvalidation checks while handling the ``FWU_SMC_IMAGE_COPY`` SMC. The SMC
524fe91230SJoel Huttonimplementation is designed to copy an image into secure memory for subsequent
534fe91230SJoel Huttonauthentication, but the vulnerabilities may allow an attacker to copy
544fe91230SJoel Huttonunexpectedly large data into secure memory. Note that a separate vulnerability
554fe91230SJoel Huttonis required to leverage these vulnerabilities; for example a way to get the
564fe91230SJoel Huttonsystem to change its behaviour based on the unexpected secure memory contents.
574fe91230SJoel Hutton
584fe91230SJoel HuttonTwo of the vulnerabilities are in the function ``bl1_fwu_image_copy()`` in
594fe91230SJoel Hutton``bl1/bl1_fwu.c``. These are listed below, referring to the v1.3 tagged version
604fe91230SJoel Huttonof the code:
614fe91230SJoel Hutton
624fe91230SJoel Hutton- Line 155:
634fe91230SJoel Hutton
644fe91230SJoel Hutton  .. code:: c
654fe91230SJoel Hutton
664fe91230SJoel Hutton    /*
674fe91230SJoel Hutton     * If last block is more than expected then
684fe91230SJoel Hutton     * clip the block to the required image size.
694fe91230SJoel Hutton     */
704fe91230SJoel Hutton    if (image_desc->copied_size + block_size >
714fe91230SJoel Hutton         image_desc->image_info.image_size) {
724fe91230SJoel Hutton        block_size = image_desc->image_info.image_size -
734fe91230SJoel Hutton            image_desc->copied_size;
744fe91230SJoel Hutton        WARN("BL1-FWU: Copy argument block_size > remaining image size."
754fe91230SJoel Hutton            " Clipping block_size\n");
764fe91230SJoel Hutton    }
774fe91230SJoel Hutton
784fe91230SJoel Hutton    /* Make sure the image src/size is mapped. */
794fe91230SJoel Hutton    if (bl1_plat_mem_check(image_src, block_size, flags)) {
804fe91230SJoel Hutton        WARN("BL1-FWU: Copy arguments source/size not mapped\n");
814fe91230SJoel Hutton        return -ENOMEM;
824fe91230SJoel Hutton    }
834fe91230SJoel Hutton
844fe91230SJoel Hutton    INFO("BL1-FWU: Continuing image copy in blocks\n");
854fe91230SJoel Hutton
864fe91230SJoel Hutton    /* Copy image for given block size. */
874fe91230SJoel Hutton    base_addr += image_desc->copied_size;
884fe91230SJoel Hutton    image_desc->copied_size += block_size;
894fe91230SJoel Hutton    memcpy((void *)base_addr, (const void *)image_src, block_size);
904fe91230SJoel Hutton    ...
914fe91230SJoel Hutton
924fe91230SJoel Hutton  This code fragment is executed when the image copy operation is performed in
934fe91230SJoel Hutton  blocks over multiple SMCs. ``block_size`` is an SMC argument and therefore
944fe91230SJoel Hutton  potentially controllable by an attacker. A very large value may result in an
954fe91230SJoel Hutton  integer overflow in the 1st ``if`` statement, which would bypass the check,
964fe91230SJoel Hutton  allowing an unclipped ``block_size`` to be passed into
974fe91230SJoel Hutton  ``bl1_plat_mem_check()``. If ``bl1_plat_mem_check()`` also passes, this may
984fe91230SJoel Hutton  result in an unexpectedly large copy of data into secure memory.
994fe91230SJoel Hutton
1004fe91230SJoel Hutton- Line 206:
1014fe91230SJoel Hutton
1024fe91230SJoel Hutton  .. code:: c
1034fe91230SJoel Hutton
1044fe91230SJoel Hutton    /* Make sure the image src/size is mapped. */
1054fe91230SJoel Hutton    if (bl1_plat_mem_check(image_src, block_size, flags)) {
1064fe91230SJoel Hutton        WARN("BL1-FWU: Copy arguments source/size not mapped\n");
1074fe91230SJoel Hutton        return -ENOMEM;
1084fe91230SJoel Hutton    }
1094fe91230SJoel Hutton
1104fe91230SJoel Hutton    /* Find out how much free trusted ram remains after BL1 load */
1114fe91230SJoel Hutton    mem_layout = bl1_plat_sec_mem_layout();
1124fe91230SJoel Hutton    if ((image_desc->image_info.image_base < mem_layout->free_base) ||
1134fe91230SJoel Hutton         (image_desc->image_info.image_base + image_size >
1144fe91230SJoel Hutton          mem_layout->free_base + mem_layout->free_size)) {
1154fe91230SJoel Hutton        WARN("BL1-FWU: Memory not available to copy\n");
1164fe91230SJoel Hutton        return -ENOMEM;
1174fe91230SJoel Hutton    }
1184fe91230SJoel Hutton
1194fe91230SJoel Hutton    /* Update the image size. */
1204fe91230SJoel Hutton    image_desc->image_info.image_size = image_size;
1214fe91230SJoel Hutton
1224fe91230SJoel Hutton    /* Copy image for given size. */
1234fe91230SJoel Hutton    memcpy((void *)base_addr, (const void *)image_src, block_size);
1244fe91230SJoel Hutton    ...
1254fe91230SJoel Hutton
1264fe91230SJoel Hutton  This code fragment is executed during the 1st invocation of the image copy
1274fe91230SJoel Hutton  operation. Both ``block_size`` and ``image_size`` are SMC arguments. A very
1284fe91230SJoel Hutton  large value of ``image_size`` may result in an integer overflow in the 2nd
1294fe91230SJoel Hutton  ``if`` statement, which would bypass the check, allowing execution to proceed.
1304fe91230SJoel Hutton  If ``bl1_plat_mem_check()`` also passes, this may result in an unexpectedly
1314fe91230SJoel Hutton  large copy of data into secure memory.
1324fe91230SJoel Hutton
1334fe91230SJoel HuttonIf the platform's implementation of ``bl1_plat_mem_check()`` is correct then it
1344fe91230SJoel Huttonmay help prevent the above 2 vulnerabilities from being exploited. However, the
1354fe91230SJoel HuttonARM platform version of this function contains a similar vulnerability:
1364fe91230SJoel Hutton
1374fe91230SJoel Hutton- Line 88 of ``plat/arm/common/arm_bl1_fwu.c`` in function of
1384fe91230SJoel Hutton  ``bl1_plat_mem_check()``:
1394fe91230SJoel Hutton
1404fe91230SJoel Hutton  .. code:: c
1414fe91230SJoel Hutton
1424fe91230SJoel Hutton    while (mmap[index].mem_size) {
1434fe91230SJoel Hutton        if ((mem_base >= mmap[index].mem_base) &&
1444fe91230SJoel Hutton            ((mem_base + mem_size)
1454fe91230SJoel Hutton            <= (mmap[index].mem_base +
1464fe91230SJoel Hutton            mmap[index].mem_size)))
1474fe91230SJoel Hutton            return 0;
1484fe91230SJoel Hutton
1494fe91230SJoel Hutton        index++;
1504fe91230SJoel Hutton    }
1514fe91230SJoel Hutton    ...
1524fe91230SJoel Hutton
1534fe91230SJoel Hutton  This function checks that the passed memory region is within one of the
1544fe91230SJoel Hutton  regions mapped in by ARM platforms. Here, ``mem_size`` may be the
1554fe91230SJoel Hutton  ``block_size`` passed from ``bl1_fwu_image_copy()``. A very large value of
1564fe91230SJoel Hutton  ``mem_size`` may result in an integer overflow and the function to incorrectly
1574fe91230SJoel Hutton  return success. Platforms that copy this insecure pattern will have the same
1584fe91230SJoel Hutton  vulnerability.
1594fe91230SJoel Hutton
16012fc6ba7SPaul Beesley.. _CVE-2016-10319: http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-10319
1614fe91230SJoel Hutton.. _48bfb88: https://github.com/ARM-software/arm-trusted-firmware/commit/48bfb88
1624fe91230SJoel Hutton.. _Pull Request #783: https://github.com/ARM-software/arm-trusted-firmware/pull/783
163