GraphicsMagick / Bugs / #531 heap-buffer-overflow in AcquireCacheNexus

#531 heap-buffer-overflow in AcquireCacheNexus

Milestone: v1.0_(example)

Status: closed-fixed

Owner: Bob Friesenhahn

Labels: None

Priority: 1

Updated: 2018-02-13

Created: 2017-12-13

Creator: Allan Zhou

Private: No

/usr/local/bin/gm mogrify -version
GraphicsMagick 1.4 snapshot-20171208 Q8 http://www.GraphicsMagick.org/
Copyright (C) 2002-2017 GraphicsMagick Group.
Additional copyrights and licenses apply to this software.
See http://www.GraphicsMagick.org/www/Copyright.html for details.

Feature Support:
Native Thread Safe yes
Large Files (> 32 bit) yes
Large Memory (> 32 bit) yes
BZIP yes
DPS no
FlashPix no
FreeType yes
Ghostscript (Library) no
JBIG no
JPEG-2000 no
JPEG yes
Little CMS no
Loadable Modules no
OpenMP yes (201511)
PNG yes
TIFF yes
TRIO no
UMEM no
WebP no
WMF no
X11 yes
XML yes
ZLIB yes

Host type: x86_64-unknown-linux-gnu

Configured using the command:
./configure 'CC=gcc' 'CXX=g++'

Final Build Parameters:
CC = gcc
CFLAGS = -fopenmp -g -fsanitize=address -Wall -pthread
CPPFLAGS = -I/usr/include/freetype2 -I/usr/include/libxml2
CXX = g++
CXXFLAGS = -pthread
LDFLAGS =
LIBS = -ltiff -lfreetype -ljpeg -lpng16 -lXext -lSM -lICE -lX11 -llzma -lbz2 -lxml2 -lz -lm -lgomp -lpthread

/usr/local/bin/gm mogrify RrJK0j36GrrtCHQ48KMqoRokb0wjIbnp.svg

=================================================================                              
==13674==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61a0000011a3 at pc 0x00000050c7b1 bp 0x7f2ceb734980 sp 0x7f2ceb734970
WRITE of size 1 at 0x61a0000011a3 thread T7                                                    
    #0 0x50c7b0 in AcquireCacheNexus magick/pixel_cache.c:925                                  
    #1 0x50ca9e in AcquireCacheViewPixels magick/pixel_cache.c:995                             
    #2 0x51138c in InterpolateViewColor magick/pixel_cache.c:2778                              
    #3 0x5577b1 in DrawAffineImage._omp_fn.0 magick/render.c:1197                              
    #4 0x7f2cf94b7d3d  (/lib64/libgomp.so.1+0x16d3d)
    #5 0x7f2cf9289608 in start_thread (/lib64/libpthread.so.0+0x7608)
    #6 0x7f2cf8fb6e6e in __clone (/lib64/libc.so.6+0x119e6e)

0x61a0000011a3 is located 0 bytes to the right of 1315-byte region [0x61a000000c80,0x61a0000011a3)
allocated by thread T0 here:                   
    #0 0x7f2cfb7f1850 in malloc (/lib64/libasan.so.4+0xde850)
    #1 0x4efc29 in MagickRealloc magick/memory.c:471
    #2 0x513694 in OpenCache magick/pixel_cache.c:3155
    #3 0x5127ef in ModifyCache magick/pixel_cache.c:2955
    #4 0x5166a7 in SetCacheNexus magick/pixel_cache.c:3891
    #5 0x516a8a in SetCacheViewPixels magick/pixel_cache.c:3970
    #6 0x516b64 in SetImagePixels magick/pixel_cache.c:4036
    #7 0x6a9b59 in ReadOnePNGImage coders/png.c:2475
    #8 0x6abfc0 in ReadPNGImage coders/png.c:2882
    #9 0x4868db in ReadImage magick/constitute.c:1607
    #10 0x9007ce in BlobToImage magick/blob.c:738
    #11 0x488cce in ReadInlineImage magick/constitute.c:2071
    #12 0x5472d6 in DrawPrimitive magick/render.c:4468
    #13 0x542541 in DrawImage magick/render.c:3424
    #14 0x7c0364 in ReadMVGImage coders/mvg.c:195
    #15 0x4868db in ReadImage magick/constitute.c:1607
    #16 0x85996e in ReadSVGImage coders/svg.c:3021
    #17 0x4868db in ReadImage magick/constitute.c:1607
    #18 0x44c0df in TransmogrifyImage magick/command.c:11714
    #19 0x44d9b2 in MogrifyImageCommand magick/command.c:12017
    #20 0x43b139 in MagickCommand magick/command.c:8872
    #21 0x467c07 in GMCommandSingle magick/command.c:17393
    #22 0x467eee in GMCommand magick/command.c:17446
    #23 0x40b916 in main utilities/gm.c:61
    #24 0x7f2cf8ebe039 in __libc_start_main (/lib64/libc.so.6+0x21039)

Thread T7 created by T0 here:
    #0 0x7f2cfb74aa2f in pthread_create (/lib64/libasan.so.4+0x37a2f)
    #1 0x7f2cf94b8309  (/lib64/libgomp.so.1+0x17309)
    #2 0x7f2cf94aecd9 in GOMP_parallel (/lib64/libgomp.so.1+0xdcd9)
    #3 0x62300000a8ff  (<unknown module>)
SUMMARY: AddressSanitizer: heap-buffer-overflow magick/pixel_cache.c:925 in AcquireCacheNexus
Shadow bytes around the buggy address:
  0x0c347fff81e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c347fff81f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c347fff8200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c347fff8210: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c347fff8220: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c347fff8230: 00 00 00 00[03]fa fa fa fa fa fa fa fa fa fa fa
  0x0c347fff8240: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c347fff8250: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c347fff8260: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c347fff8270: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c347fff8280: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==13674==ABORTING

1 Attachments

RrJK0j36GrrtCHQ48KMqoRokb0wjIbnp.svg

Discussion

Allan Zhou - 2017-12-14

https://github.com/henices/pocs/raw/master/RrJK0j36GrrtCHQ48KMqoRokb0wjIbnp.svg.zip

Credit: zz of Nsfocus Security Team

Last edit: Allan Zhou 2018-01-10

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Bob Friesenhahn - 2017-12-17

assigned_to: Bob Friesenhahn
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Bob Friesenhahn - 2018-01-07

status: open --> closed-fixed
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Bob Friesenhahn - 2018-01-07

This problem is fixed in Mercurial changeset 15322:b41e2efce6d3. The problem afflicts all GraphicsMagick 1.3.X releases. Thank you for reporting this issue.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

el_cubano - 2018-02-13

The fix (b41e2efce6d3) and the subsequent commit (d30ed06e9b87), which appears to be required as well, changes the return type of InterpolateViewColor(). I am preparing a security update for Debian and I am concerned that this change breaks the ABI. I have started a thread on the debian-lts mailing list here: https://lists.debian.org/debian-lts/2018/02/msg00047.html

I would appreciate some upstream feedback.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- Bob Friesenhahn - 2018-02-13
  
  On Tue, 13 Feb 2018, el_cubano wrote:
  
  The fix (b41e2efce6d3) and the subsequent commit (d30ed06e9b87), which appears to be required as well, changes the return type of InterpolateViewColor(). I am preparing a security update for Debian and I am concerned that this change breaks the ABI. I have started a thread on the debian-lts mailing list here: https://lists.debian.org/debian-lts/2018/02/msg00047.html
  
  I would appreciate some upstream feedback.
  
  I don't believe that changing a void return to an unsigned int return
  changes the shared library ABI. I would be interested to see evidence
  to the contrary.
  https://stackoverflow.com/questions/15626579/c-abi-is-it-safe-to-change-void-function-to-return-int
  
  Furthermore, the prototype for this function is only exposed when
  MAGICK_IMPLEMENTATION is defined, which should only be defined when
  compiling GraphicsMagick itself. This means that this function should
  not be referenced by external code.
  
  It would be better if these internal implementation functions were not
  exported by the shared library at all. It would be better if the
  prototypes for internal implementation functions could not be enabled
  via a define by a rogue user.
  
  Perhaps (and even likely) internal functions will no longer be exposed
  by the shared library in the future and then more interesting
  discussions can ensue about ABI.
  
  Bob
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
  - el_cubano - 2018-02-13
    
    Bob,
    
    Thanks for the information. I did not think deeply enough on this. I will tweak the package prior to uploading it.
    
    If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
    - Bob Friesenhahn - 2018-02-13
      
      On Tue, 13 Feb 2018, el_cubano wrote:
      
      Thanks for the information. I did not think deeply enough on this. I will tweak the package prior to uploading it.
      
      At one time, C did not support prototypes, and when prototypes were
      added it was decided that the default return type should be an 'int'
      in the absence of a prototype. I think that this means that the
      language implementation needs to provide support for an integer return
      value (in a register) even if the function does not provide a return
      value.
      
      Bob
      
      Bob Friesenhahn
      bfriesen@simple.dallas.tx.us, http://www.simplesystems.org/users/bfriesen/
      GraphicsMagick Maintainer, http://www.GraphicsMagick.org/
      
      If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

heap-buffer-overflow in AcquireCacheNexus

Swiss army knife of image processing

Group

Searches

Help

#531 heap-buffer-overflow in AcquireCacheNexus

Discussion

Bob