Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

heap use after free in RenderFreetype #781

Closed
noirfate opened this issue Sep 25, 2017 · 5 comments
Closed

heap use after free in RenderFreetype #781

noirfate opened this issue Sep 25, 2017 · 5 comments
Labels

Comments

@noirfate
Copy link

noirfate commented Sep 25, 2017

Version : ImageMagick 7.0.7-4 Q16 x86_64 2017-09-22 http://www.imagemagick.org

In order to reproduce this bug, need to build ImageMagick and Freetype2 with ASAN.

Add a crafted font in ~/.config/ImageMagick/type.xml

<type
     format="ttf"
     name="test"
     fullname="Z003 Medium Italic"
     family="Z003"
     glyphs="/root/out/crashes/test.ttf"
     />

The crafted font file : https://github.com/noirfate/test/blob/master/test1.ttf

After have added the crafted font, run :

magick -background lightblue -fill blue -font test -size 480x360 caption:hello world 1.gif

ASAN would report :

==3531==ERROR: AddressSanitizer: heap-use-after-free on address 0x6080000092a0 at pc 0x7f31ab1cc934 bp 0x7ffd03c83330 sp 0x7ffd03c83328
READ of size 8 at 0x6080000092a0 thread T0
    #0 0x7f31ab1cc933 in FT_Done_Glyph /root/dep_src/freetype2/src/base/ftglyph.c:637:46
    #1 0x7f31ac530d4c in RenderFreetype /root/dep_src/ImageMagick-master/MagickCore/annotate.c:1797:7
    #2 0x7f31ac529114 in RenderType /root/dep_src/ImageMagick-master/MagickCore/annotate.c:1027:10
    #3 0x7f31ac527944 in GetTypeMetrics /root/dep_src/ImageMagick-master/MagickCore/annotate.c:906:10
    #4 0x7f31ac52aa77 in FormatMagickCaption /root/dep_src/ImageMagick-master/MagickCore/annotate.c:627:12
    #5 0x7f31ac99ea52 in ReadCAPTIONImage /root/dep_src/ImageMagick-master/coders/caption.c:221:11
    #6 0x7f31ac5c8688 in ReadImage /root/dep_src/ImageMagick-master/MagickCore/constitute.c:497:13
    #7 0x7f31ac5cad54 in ReadImages /root/dep_src/ImageMagick-master/MagickCore/constitute.c:866:9
    #8 0x7f31abf87ad9 in CLINoImageOperator /root/dep_src/ImageMagick-master/MagickWand/operation.c:4760:22
    #9 0x7f31abf8a1fc in CLIOption /root/dep_src/ImageMagick-master/MagickWand/operation.c:5255:7
    #10 0x7f31abe84577 in ProcessCommandOptions /root/dep_src/ImageMagick-master/MagickWand/magick-cli.c:424:13
    #11 0x7f31abe853e0 in MagickImageCommand /root/dep_src/ImageMagick-master/MagickWand/magick-cli.c:794:5
    #12 0x7f31abeb7ad7 in MagickCommandGenesis /root/dep_src/ImageMagick-master/MagickWand/mogrify.c:183:14
    #13 0x4ee16d in MagickMain /root/dep_src/ImageMagick-master/utilities/magick.c:149:10
    #14 0x4ee16d in main /root/dep_src/ImageMagick-master/utilities/magick.c:180
    #15 0x7f31a30f482f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291
    #16 0x41a2d8 in _start (/root/deps/bin/magick+0x41a2d8)

0x6080000092a0 is located 0 bytes inside of 88-byte region [0x6080000092a0,0x6080000092f8)
freed by thread T0 here:
    #0 0x4c0c8b in __interceptor_free /home/snd-local/releases/4.0.1/release/final/llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:47:3
    #1 0x7f31ac530ad0 in RenderFreetype /root/dep_src/ImageMagick-master/MagickCore/annotate.c:1772:5
    #2 0x7f31ac529114 in RenderType /root/dep_src/ImageMagick-master/MagickCore/annotate.c:1027:10
    #3 0x7f31ac527944 in GetTypeMetrics /root/dep_src/ImageMagick-master/MagickCore/annotate.c:906:10
    #4 0x7f31ac52aa77 in FormatMagickCaption /root/dep_src/ImageMagick-master/MagickCore/annotate.c:627:12
    #5 0x7f31ac99ea52 in ReadCAPTIONImage /root/dep_src/ImageMagick-master/coders/caption.c:221:11
    #6 0x7f31ac5c8688 in ReadImage /root/dep_src/ImageMagick-master/MagickCore/constitute.c:497:13
    #7 0x7f31ac5cad54 in ReadImages /root/dep_src/ImageMagick-master/MagickCore/constitute.c:866:9
    #8 0x7f31abf87ad9 in CLINoImageOperator /root/dep_src/ImageMagick-master/MagickWand/operation.c:4760:22
    #9 0x7f31abf8a1fc in CLIOption /root/dep_src/ImageMagick-master/MagickWand/operation.c:5255:7
    #10 0x7f31abe84577 in ProcessCommandOptions /root/dep_src/ImageMagick-master/MagickWand/magick-cli.c:424:13

previously allocated by thread T0 here:
    #0 0x4c0fdc in __interceptor_malloc /home/snd-local/releases/4.0.1/release/final/llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:66:3
    #1 0x7f31ab1952e2 in ft_mem_qalloc /root/dep_src/freetype2/src/base/ftutil.c:76:15
    #2 0x7f31ab1952e2 in ft_mem_alloc /root/dep_src/freetype2/src/base/ftutil.c:55
    #3 0x7f31ac52f508 in RenderFreetype /root/dep_src/ImageMagick-master/MagickCore/annotate.c:1613:15
    #4 0x7f31ac529114 in RenderType /root/dep_src/ImageMagick-master/MagickCore/annotate.c:1027:10
    #5 0x7f31ac527944 in GetTypeMetrics /root/dep_src/ImageMagick-master/MagickCore/annotate.c:906:10
    #6 0x7f31ac52aa77 in FormatMagickCaption /root/dep_src/ImageMagick-master/MagickCore/annotate.c:627:12
    #7 0x7f31ac99ea52 in ReadCAPTIONImage /root/dep_src/ImageMagick-master/coders/caption.c:221:11
    #8 0x7f31ac5c8688 in ReadImage /root/dep_src/ImageMagick-master/MagickCore/constitute.c:497:13
    #9 0x7f31ac5cad54 in ReadImages /root/dep_src/ImageMagick-master/MagickCore/constitute.c:866:9
    #10 0x7f31abf87ad9 in CLINoImageOperator /root/dep_src/ImageMagick-master/MagickWand/operation.c:4760:22
    #11 0x7f31abf8a1fc in CLIOption /root/dep_src/ImageMagick-master/MagickWand/operation.c:5255:7
    #12 0x7f31abe84577 in ProcessCommandOptions /root/dep_src/ImageMagick-master/MagickWand/magick-cli.c:424:13

After I took a look at the code, I think it maybe caused by calling FT_Done_Glyph multiple times.

First when last_glyph.id != 0, it will enter the if condition

annotate.c:1762
    if (last_glyph.id != 0)
      FT_Done_Glyph(last_glyph.image);
      last_glyph=glyph;
      code=GetUTFCode(p+grapheme[i].cluster);
  }

Thus last_glyph is equal to glyph.

And then, in some cases, it will call FT_Done_Glyph(last_glyph.image) and FT_Done_Glyph(glyph.image) both. Since last_glyph = glyph, the last call will trigger this bug.

  if (last_glyph.id != 0)
    FT_Done_Glyph(last_glyph.image);
  /*
    Determine font metrics.
  */
  glyph.id=FT_Get_Char_Index(face,'_');
  glyph.origin=origin;
  ft_status=FT_Load_Glyph(face,glyph.id,flags);
  if (ft_status == 0)
  {
     ... 
      FT_Done_Glyph(glyph.image);
    }
@mikayla-grace
Copy link

Unfortunately, we cannot reproduce the problem you reported. We're using ImageMagick 7.0.7-4 (ASN-enabled) and Freetype 2.7.1. When we run your command, we get:

$ magick -background lightblue -fill blue -font test -size 480x360 caption:'hello world' 1.gif
magick: unable to read font `/root/test1.ttf' @ error/annotate.c/RenderFreetype/1466.

and if we remove that font, ASN does not return any memory leaks:

$ magick -background lightblue -fill blue -font arial -size 480x360 caption:'hello world' 1.gif

What version fo Freetype are you using?

Regarding your conjecture about freeing the same glyph image. Possible of course but debugging does not reveal a problem. Notice we call FT_Get_Glyph() before we call FT_Done_Glyph(glyph.image). That should produce a difference image instance-- and debugging suggests it does.

@noirfate
Copy link
Author

I tried freetype 2.7.1, this bug occurs as well. You need to compile freetype with ASAN, otherwise ASAN can't discover the bug.

You said that it calls FT_Get_Glyph() before call FT_Done_Glyph(), I agree with you. But the case is when FT_Get_Glyph() is failed, it still calls FT_Done_Glyph().

Below is my debug info :

1778      ft_status=FT_Load_Glyph(face,glyph.id,flags);
(gdb) 
1779      if (ft_status == 0)
(gdb) 
1781          ft_status=FT_Get_Glyph(face->glyph,&glyph.image);
(gdb) 
1782          if (ft_status == 0)
(gdb) 
1797          FT_Done_Glyph(glyph.image);
(gdb) 
=================================================================
==22696==ERROR: AddressSanitizer: heap-use-after-free on address 0x60800000aea0 at pc 0x7ffff5a7d934 bp 0x7ffffffef330 sp 0x7ffffffef328
READ of size 8 at 0x60800000aea0 thread T0
    #0 0x7ffff5a7d933 in FT_Done_Glyph /root/dep_src/freetype2/src/base/ftglyph.c:637:46
    #1 0x7ffff6de1c8c in RenderFreetype /root/dep_src/ImageMagick-master/MagickCore/annotate.c:1797:7
    #2 0x7ffff6dda054 in RenderType /root/dep_src/ImageMagick-master/MagickCore/annotate.c:1027:10
    #3 0x7ffff6dd8884 in GetTypeMetrics /root/dep_src/ImageMagick-master/MagickCore/annotate.c:906:10
    #4 0x7ffff6ddb9b7 in FormatMagickCaption /root/dep_src/ImageMagick-master/MagickCore/annotate.c:627:12
    #5 0x7ffff724ef32 in ReadCAPTIONImage /root/dep_src/ImageMagick-master/coders/caption.c:221:11
    #6 0x7ffff6e78ae8 in ReadImage /root/dep_src/ImageMagick-master/MagickCore/constitute.c:497:13
    #7 0x7ffff6e7b1b4 in ReadImages /root/dep_src/ImageMagick-master/MagickCore/constitute.c:866:9
    #8 0x7ffff6838ad9 in CLINoImageOperator /root/dep_src/ImageMagick-master/MagickWand/operation.c:4760:22
    #9 0x7ffff683b1fc in CLIOption /root/dep_src/ImageMagick-master/MagickWand/operation.c:5255:7
    #10 0x7ffff6735577 in ProcessCommandOptions /root/dep_src/ImageMagick-master/MagickWand/magick-cli.c:424:13
    #11 0x7ffff67363e0 in MagickImageCommand /root/dep_src/ImageMagick-master/MagickWand/magick-cli.c:794:5
    #12 0x7ffff6768ad7 in MagickCommandGenesis /root/dep_src/ImageMagick-master/MagickWand/mogrify.c:183:14
    #13 0x4ee16d in MagickMain /root/dep_src/ImageMagick-master/utilities/magick.c:149:10
    #14 0x4ee16d in main /root/dep_src/ImageMagick-master/utilities/magick.c:180
    #15 0x7fffed9a582f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291
    #16 0x41a2d8 in _start (/root/deps/bin/magick+0x41a2d8)

The reason is when ft_status != 0, it still calls FT_Done_Glyph().

if (ft_status == 0)
{
   ft_status=FT_Get_Glyph(face->glyph,&glyph.image);
   if (ft_status == 0)
   {
      ...
   }
   // I think when ft_status !=0, it should not call FT_Done_Glyph, the fix maybe move FT_Done_Glyph()    
   into the if(ft_status == 0)
   FT_Done_Glyph(glyph.image);
}

@mikayla-grace
Copy link

Thanks for the problem report. We can reproduce it and will have a patch to fix it in GIT master branch @ https://github.com/ImageMagick/ImageMagick later today. The patch will be available in the beta releases of ImageMagick @ http://www.imagemagick.org/download/beta/ by sometime tomorrow.

@nohmask
Copy link

nohmask commented Oct 2, 2017

This was assigned CVE-2017-14989

@dlemstra dlemstra added the bug label Oct 3, 2017
@dlemstra dlemstra closed this as completed Oct 3, 2017
@fgeek
Copy link

fgeek commented Oct 9, 2017

@noirfate did you use afl to find this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

5 participants