1672223 - (CVE-2020-15999) Heap buffer overflow due to integer truncation in FreeType

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Daniel Veditz [:dveditz]]

Attachment: crbug1139963.html

The Google Chrome team is fixing a heap buffer overflow in freetype. Although our library is called FreeType2 the affected code appears the same. I have not yet tested this in an ASAN build to verify if we are affected, but according to Google Project Zero they have found this being exploited in the wild (against Chrome, presumably)

Calling this sec-critical for now assuming we're affected, and setting the security group to the least-restrictive "Release Track" so it's visible to more people if we have to chemspill.

CREDIT: Sergei Glazunov of Google Project Zero

• We have evidence that this bug is being used in the wild. *
• Therefore, this bug is subject to a 7 day disclosure deadline. *

(it was reported to Chrome yesterday, so 6 days left)

From crbug 1139963:

A vulnerability exists in the function Load_SBit_Png, which processes PNG
images embedded into fonts. This function:

1. Obtains the image width and height from the header as 32-bit integers.
2. Truncates the obtained values to 16 bit and stores them in a
   TT_SBit_Metrics structure.
3. Uses the truncated values to calculate the bitmap size.
4. Allocates the backing store of that size.
5. Passes png_struct and the backing store to a libpng function.

The issue is that libpng uses the original 32-bit values, which are saved in
png_struct. Therefore, if the original width and/or height are greater than
65535, the allocated buffer won't be able to fit the bitmap

FT_LOCAL_DEF( FT_Error )
  Load_SBit_Png( FT_GlyphSlot     slot,
                 FT_Int           x_offset,
                 FT_Int           y_offset,
                 FT_Int           pix_bits,
                 TT_SBit_Metrics  metrics,
                 FT_Memory        memory,
                 FT_Byte*         data,
                 FT_UInt          png_len,
                 FT_Bool          populate_map_and_metrics,
                 FT_Bool          metrics_only )
  {

[...]

    png_get_IHDR( png, info,
                  &imgWidth, &imgHeight,
                  &bitdepth, &color_type, &interlace,
                  NULL, NULL ); // *** 1 ***

[...]

    if ( populate_map_and_metrics )
    {
      metrics->width  = (FT_UShort)imgWidth; // *** 2 ***
      metrics->height = (FT_UShort)imgHeight;

      map->width      = metrics->width;
      map->rows       = metrics->height;
      map->pixel_mode = FT_PIXEL_MODE_BGRA;
      map->pitch      = (int)( map->width * 4 );

[...]

    if ( populate_map_and_metrics )
    {
      /* this doesn't overflow: 0x7FFF * 0x7FFF * 4 < 2^32 */
      FT_ULong  size = map->rows * (FT_ULong)map->pitch; // *** 3 ***


      error = ft_glyphslot_alloc_bitmap( slot, size ); // *** 4 ***
      if ( error )
        goto DestroyExit;
    }

[...]

    png_read_image( png, rows ); // *** 5 ***

Comment 1

[Ryan VanderMeulen [:RyanVM]]

In the announcement that went out for the 2.10.4 release containing this fix, they said if affects anybody who has FT_CONFIG_OPTION_USE_PNG set. AFAICT, that includes us?
https://searchfox.org/mozilla-central/source/modules/freetype2/moz.build#88

Comment 2

[Jonathan Kew [:jfkthame]]

As far as I'm aware, Load_SBit_Png will be used for opentype fonts that contain embedded bitmaps (such as Noto Color Emoji), but by default we strip those tables from webfont resources so they can't be used via @font-face, they only work when installed locally.

There's a pref gfx.downloadable_fonts.keep_color_bitmaps that controls this, so it's possible for a user to open up the vulnerability, but it's false by default:
https://searchfox.org/mozilla-central/rev/61728de8273c04fe2417c475fc0637e8b79210d7/modules/libpref/init/StaticPrefList.yaml#4180-4186

As such, I think we should be safe from this with default prefs settings.

Comment 3

[Daniel Veditz [:dveditz]]

Chrome's patch is https://chromium.googlesource.com/chromium/src/third_party/freetype2.git/+/80389cab7f5823e7c3b3fe4190a7c337439317de

Comment 4

[Daniel Veditz [:dveditz]]

thanks for the pref tip. Even with that set I can't get a crash in Windows ASAN Nightly with the testcase. With or without the pref flip I do see the following console warnings so maybe the testcase isn't quite right for Firefox?

downloadable font: bad search range (font-family: "fontname" style:normal weight:400 stretch:100 src index:0) source: (invalid URI)
downloadable font: bad range shift (font-family: "fontname" style:normal weight:400 stretch:100 src index:0) source: (invalid URI)
downloadable font: maxp: Bad maxZones: 0 (font-family: "fontname" style:normal weight:400 stretch:100 src index:0) source: (invalid URI)

Comment 5

[Jonathan Kew [:jfkthame]]

It wouldn't affect Windows, we don't use freetype there -- only on Linux and Android.

Having said that, I don't get a crash on Linux even with the pref toggled. (Opt build; maybe ASAN or valgrind would show something, but I don't have those builds on hand immediately.)

Comment 6

[Daniel Veditz [:dveditz]]

I'll lower this to sec-moderate based on the non-default pref being required. We shouldn't have to re-spin Fx82 for this.

Do we have any hints how popular an option gfx.downloadable_fonts.keep_color_bitmaps is? I bet fairly popular in some communities (discussion boards that use that kind of font?) but maybe not all that widespread. I only get a couple of Google hits for sites telling people how they can use the pref to enable emojis.

Comment 7

[Jonathan Kew [:jfkthame]]

We don't have any actual data afaik, but I doubt it's at all common. There aren't a huge number of such fonts out there besides Noto Color Emoji, which generally comes preinstalled on devices, and is rather big to deploy as a webfont anyhow.

People doing stuff with color webfonts are more often using the outline-based COLR/CPAL format, or sometimes SVG-in-OT, which are unrelated to this issue and don't depend on setting the keep_color_bitmaps pref.

So I don't see this justifying a respin or other highly-urgent response. Given that it's a nice simple patch, I think it'd make sense to cherry-pick it for beta/esr releases, though.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Attachment: Bug 1672223 - [sfnt] Fix heap buffer overflow. r=jfkthame

This is CVE-2020-15999.

• src/sfnt/pngshim.c (Load_SBit_Png): Test bitmap size earlier.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

FWIW, the cherry-pick grafts cleanly to all branches.

Comment 10

[Daniel Veditz [:dveditz]]

The upstream bug and patch can be found at https://savannah.nongnu.org/bugs/?59308 -- looks like the issue is now essentially public? We should keep our bug hidden as long as the chrome bug is hidden though.

Comment 11

[Gian-Carlo Pascutto [:gcp]]

Google Project Zero is now tweeting about this: https://twitter.com/benhawkes/status/1318640422571266048

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Pushed to autoland:
https://hg.mozilla.org/integration/autoland/rev/5c356dd9b0f31f612ce90b1f35b14d8cdb0b4ecb

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/5c356dd9b0f3

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9182701 [details]
Bug 1672223 - [sfnt] Fix heap buffer overflow. r=jfkthame

Beta/Release Uplift Approval Request

• User impact if declined: Publicly-disclosed and exploited security vulnerability
• Is this code covered by automated tests?: Unknown
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Only affects Linux and Android users with a non-default pref set.
• String changes made/needed:

Comment 15

[Matthew Finkel [:sysrqb]]

(In reply to Jonathan Kew (:jfkthame) from comment #5)

It wouldn't affect Windows, we don't use freetype there -- only on Linux and Android.

Can I clarify that Android is affected due to in-tree freetype2, but Linux is only affected when the installed system library is vulnerable? Maybe I'm missing where the vendored version is used for Linux, too.

Comment 16

[Jonathan Kew [:jfkthame]]

Right, I should have added that on Linux, we don't (AFAIK) use in-tree freetype and so the status here would depend on the system library and not on what we do with our vendored copy.

Comment 17

[Julien Cristau [:jcristau]]

I assume the pref is not exposed to users on fenix (where about:config is nightly only, AIUI).

Comment 18

[Daniel Veditz [:dveditz]]

Comment on attachment 9182701 [details]
Bug 1672223 - [sfnt] Fix heap buffer overflow. r=jfkthame

a=dveditz

Normally I'd say don't land on ESR this far before release, but in this case with GPZ publicizing the bug there's no harm done.

Comment 19

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/bdeb2bb120e8

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/d643964ea42f

Comment 21

[Andrew McCreight [:mccr8]]

Project Zero issue is public now (same content as comment 0):

https://bugs.chromium.org/p/project-zero/issues/detail?id=2103

Comment 22

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: advisory.txt

Comment 23

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: advisory.txt

Comment 24

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: advisory.txt