Microsoft DirectWrite / AFDKO insufficient integer overflow check in dnaGrow 



-----=====[ Background ]=====-----

AFDKO (Adobe Font Development Kit for OpenType) is a set of tools for examining, modifying and building fonts. The core part of this toolset is a font handling library written in C, which provides interfaces for reading and writing Type 1, OpenType, TrueType (to some extent) and several other font formats. While the library existed as early as 2000, it was open-sourced by Adobe in 2014 on GitHub [1, 2], and is still actively developed. The font parsing code can be generally found under afdko/c/public/lib/source/*read/*.c in the project directory tree.

At the time of this writing, based on the available source code, we conclude that AFDKO was originally developed to only process valid, well-formatted font files. It contains very few to no sanity checks of the input data, which makes it susceptible to memory corruption issues (e.g. buffer overflows) and other memory safety problems, if the input file doesn't conform to the format specification.

We have recently discovered that starting with Windows 10 1709 (Fall Creators Update, released in October 2017), Microsoft's DirectWrite library [3] includes parts of AFDKO, and specifically the modules for reading and writing OpenType/CFF fonts (internally called cfr/cfw). The code is reachable through dwrite!AdobeCFF2Snapshot, called by methods of the FontInstancer class, called by dwrite!DWriteFontFace::CreateInstancedStream and dwrite!DWriteFactory::CreateInstancedStream. This strongly indicates that the code is used for instancing the relatively new variable fonts [4], i.e. building a single instance of a variable font with a specific set of attributes. The CreateInstancedStream method is not a member of a public COM interface, but we have found that it is called by d2d1!dxc::TextConvertor::InstanceFontResources, which led us to find out that it can be reached through the Direct2D printing interface. It is unclear if there are other ways to trigger the font instancing functionality.

One example of a client application which uses Direct2D printing is Microsoft Edge. If a user opens a specially crafted website with an embedded OpenType variable font and decides to print it (to PDF, XPS, or another physical or virtual printer), the AFDKO code will execute with the attacker's font file as input.

-----=====[ Description ]=====-----

The AFDKO library has its own implementation of dynamic arrays, semantically resembling e.g. std::vector from C++. These objects are implemented in c/public/lib/source/dynarr/dynarr.c and c/public/lib/api/dynarr.h. One of the more important functions operating on dynarrays is dnaGrow(), designed to extend a dynamic array or set its initial size. It is used by numerous other routines such as dnaSetCnt, dnaIndex, dnaMax, dnaNext, dnaExtend, or the dnaSET_CNT macro. It should be noted that it is potentially possible for the function's arguments to be unsanitized values loaded directly from an input font.

In order to prevent security vulnerabilities stemming from arithmetic errors while calculating buffer lengths, dnaGrow() explicitly checks for integer overflow conditions. One example of such check is shown below:

--- cut ---
    79      } else if (da->size == 0) {
    80          /* Initial allocation */
    81          size_t init = (size_t)da->array;
    82          size_t new_mem_size;
    83          new_size = ((size_t)index < init) ? init : init + ((index - init) + da->incr) / da->incr * da->incr;
    84          new_mem_size = new_size * elemsize;
    85          if (new_mem_size / elemsize == new_size) /* check math overflow */
    86              new_ptr = h->mem.manage(&h->mem, NULL, new_mem_size);
    87          else
    88              new_ptr = NULL;
    89      } else {
--- cut ---

The if statement in line 85 guarantees that \"new_mem_size\" is exactly the product of \"new_size\" and \"elemsize\", and an integer overflow doesn't take place. There is also a similar check in another code branch a few lines below:

--- cut ---
    89      } else {
    90          /* Incremental allocation */
    91          new_size = da->size +
    92                     ((index - da->size) + da->incr) / da->incr * da->incr;
    93          if (new_size * elemsize >= new_size) /* check math overflow */
    94              new_ptr = h->mem.manage(&h->mem, da->array, new_size * elemsize);
    95          else
    96              new_ptr = NULL;
    97      }
--- cut ---

Here, the overflow check in line 93 is incorrect. It is possible to craft the \"new_size\" and \"elemsize\" values such that an undetected integer overflow takes place. For instance, on 32-bit platforms, if new_size=0x58000000 and elemsize=0x4, then (size_t)(new_size * elemsize) is 0x60000000, truncated from the actual result of 0x160000000. These example values pass the above sanity check, but still lead to the allocation of a too small buffer in relation to the requested number of elements.

While there is an evident problem in the code, we haven't found an obvious way to exploit it after a brief analysis. We believe that this is mostly caused by the fact that the faulty code is found in a code branch responsible for incrementally extending the size of a non-empty array, and not the one performing the initial allocation. There are few arrays which are grown dynamically during the library run time; most of them have their length set only once and never changed later on. It is even more difficult to find a dynamically extended array which accepts completely arbitrary element counts.

Nevertheless, given the right set of conditions (currently or in the future), this bug might facilitate the exploitation of an integer overflow condition that would otherwise be impossible to trigger. As such an integer overflow could subsequently lead to memory corruption, we decided to report the problem despite the apparent lack of an attack vector at this moment. We recommend fixing it by using the correct overflow check construct from line 85.

-----=====[ References ]=====-----

[1] https://blog.typekit.com/2014/09/19/new-from-adobe-type-open-sourced-font-development-tools/
[2] https://github.com/adobe-type-tools/afdko
[3] https://docs.microsoft.com/en-us/windows/desktop/directwrite/direct-write-portal
[4] https://medium.com/variable-fonts/https-medium-com-tiro-introducing-opentype-variable-fonts-12ba6cd2369

This bug is subject to a 90 day disclosure deadline. After 90 days elapse or a patch has been made broadly available (whichever is earlier), the bug report will become visible to the public.


Found by: mjurczyk@google.com