heap-buffer-overflow in WritePNMImage of coders/pnm.c

[galycannon]

Prerequisites

• I have written a descriptive issue title
• I have verified that I am using the latest version of ImageMagick
• I have searched open and closed issues to ensure it has not already been reported

Description

There is a heap buffer overflow vulnerability in function WritePNMImage of coders/pnm.c.

Steps to Reproduce

poc
magick convert $poc ./test.pnm
=================================================================
==16178==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000b6f0 at pc 0x0000009f987d bp 0x7fff206e34b0 sp 0x7fff206e34a0
READ of size 8 at 0x60200000b6f0 thread T0
==16178==AddressSanitizer: while reporting a bug found another one. Ignoring.
#0 0x9f987c in SetGrayscaleImage._omp_fn.4 MagickCore/quantize.c:3453
#1 0x7f12195becbe in GOMP_parallel (/usr/lib/x86_64-linux-gnu/libgomp.so.1+0xbcbe)
#2 0x9f676d in SetGrayscaleImage MagickCore/quantize.c:3432
#3 0x9f3ee7 in QuantizeImage MagickCore/quantize.c:2677
#4 0x7cf518 in SetImageType MagickCore/attribute.c:1364
#5 0x6766a7 in WritePNMImage coders/pnm.c:1938
#6 0x8485ba in WriteImage MagickCore/constitute.c:1159
#7 0x8492df in WriteImages MagickCore/constitute.c:1376
#8 0xbf05a1 in ConvertImageCommand MagickWand/convert.c:3305
#9 0xcdc6f2 in MagickCommandGenesis MagickWand/mogrify.c:184
#10 0x410091 in MagickMain utilities/magick.c:149
#11 0x410272 in main utilities/magick.c:180
#12 0x7f1218cea82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
#13 0x40fba8 in _start (/home/ImageMagick/utilities/magick+0x40fba8)
`0x60200000b6f4 is located 0 bytes to the right of 4-byte region [0x60200000b6f0,0x60200000b6f4)` `allocated by thread T0 here:` ` #0 0x7f121d489602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)` ` #1 0x4408a5 in AcquireMagickMemory MagickCore/memory.c:478` ` #2 0x4408f9 in AcquireQuantumMemory MagickCore/memory.c:551` ` #3 0x4c3f69 in ConstantString MagickCore/string.c:713` ` #4 0x43d3e0 in AcquireMagickInfo MagickCore/magick.c:193` ` #5 0x6747aa in RegisterPNMImage coders/pnm.c:1456` ` #6 0x49fc1d in RegisterStaticModule MagickCore/static.c:257` ` #7 0x43df66 in GetMagickInfo MagickCore/magick.c:642` ` #8 0x42164a in SetImageInfo MagickCore/image.c:2855` ` #9 0x849087 in WriteImages MagickCore/constitute.c:1336` ` #10 0xbf05a1 in ConvertImageCommand MagickWand/convert.c:3305` ` #11 0xcdc6f2 in MagickCommandGenesis MagickWand/mogrify.c:184` ` #12 0x410091 in MagickMain utilities/magick.c:149` ` #13 0x410272 in main utilities/magick.c:180` ` #14 0x7f1218cea82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)`
SUMMARY: AddressSanitizer: heap-buffer-overflow MagickCore/quantize.c:3453 SetGrayscaleImage._omp_fn.4
Shadow bytes around the buggy address:
0x0c047fff9680: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c047fff9690: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c047fff96a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c047fff96b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c047fff96c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c047fff96d0: fa fa fa fa fa fa 00 00 fa fa 04 fa fa fa[04]fa
0x0c047fff96e0: fa fa 00 00 fa fa 04 fa fa fa 04 fa fa fa 04 fa
0x0c047fff96f0: fa fa 04 fa fa fa 04 fa fa fa 04 fa fa fa 04 fa
0x0c047fff9700: fa fa 04 fa fa fa 04 fa fa fa 04 fa fa fa 00 04
0x0c047fff9710: fa fa 00 04 fa fa 00 04 fa fa 00 04 fa fa 00 04
0x0c047fff9720: fa fa 00 04 fa fa 00 04 fa fa 00 04 fa fa 00 04
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
Heap right redzone: fb
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack partial redzone: f4
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
==16178==ABORTING

System Configuration

• ImageMagick version:

Version: ImageMagick 7.0.8-40 Q16 x86_64 2019-04-08 https://imagemagick.org
Copyright: ? 1999-2019 ImageMagick Studio LLC
License: https://imagemagick.org/script/license.php
Features: Cipher DPC HDRI OpenMP 
Delegates (built-in): bzlib djvu fftw fontconfig freetype jbig jng jpeg lcms lqr lzma openexr pangocairo png tiff wmf x xml zlib

• Environment (Operating system, version and so on):

Distributor ID:	Ubuntu
Description:	Ubuntu 16.04.1 LTS
Release:	16.04
Codename:	xenial

• Additional information:

[urban-warrior]

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 @ https://www.imagemagick.org/download/beta/ by sometime tomorrow.

[abergmann]

CVE-2019-11598 was assigned to this issue.

[hlef]

@urban-warrior : I think there are a few weaknesses in this patch. Shouldn't MaxMap be MaxMap+1 ? This looks like a potential overflow issue to me.

Also, the memset call initializes MaxColormapSize array elements, not MagickMax(MaxColormapSize+1, MaxMap). This looks wrong to me, is this intentional?

Thanks for your work!

[urban-warrior]

Your analysis is correct. We'll add a patch to correct the allocation and initialization of colormap_index.

[hlef]

Your analysis is correct. We'll add a patch to correct the allocation and initialization of colormap_index.

thanks!

One question though: what is the exact purpose of the SyncAuthenticPixels() call in coders/exr.c? Is this only an optimization which leads pixels to be written earlier and in smaller steps to persistent storage?

I am currently backporting this patch to older versions of ImageMagick and would like to know what belongs to the security patch and what not.

[urban-warrior]

SyncAuthenticPixels() ensures the pixel cache backing store is sync with any pixels in the staging buffer. In most cases the staging buffer and the pixel cache point to the same memory location, but not always. See https://imagemagick.org/script/architecture.php#cache for the pixel cache rules of engagement.