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-buffer-overflow at MagickCore/statistic.c:559:43 in EvaluateImages #1615

Closed
3 tasks done
SuhwanSong opened this issue Jun 22, 2019 · 6 comments
Closed
3 tasks done
Labels
Milestone

Comments

@SuhwanSong
Copy link

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's a heap-buffer-overflow at MagickCore/statistic.c:559:43 in EvaluateImages.

Steps to Reproduce

run_cmd:
magick -seed 0 -monitor -bias 63 "(" magick:rose -colorize 172,35,77 ")" "(" magick:logo +repage ")" -crop 507x10'!'+20-54 -evaluate-sequence Median tmp

Here's ASAN log.

=================================================================
==10817==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x611000000c80 at pc 0x7f0648490e06 bp 0x7ffce3c96900 sp 0x7ffce3c968f8
WRITE of size 8 at 0x611000000c80 thread T0
    #0 0x7f0648490e05 in EvaluateImages MagickCore/statistic.c:559:43
    #1 0x7f0647aa55bf in CLIListOperatorImages MagickWand/operation.c:4084:22
    #2 0x7f0647aaf35e in CLIOption MagickWand/operation.c:5279:14
    #3 0x7f06478f0a99 in ProcessCommandOptions MagickWand/magick-cli.c:477:7
    #4 0x7f06478f1d0a in MagickImageCommand MagickWand/magick-cli.c:796:5
    #5 0x7f064793bba1 in MagickCommandGenesis MagickWand/mogrify.c:185:14
    #6 0x526f95 in MagickMain utilities/magick.c:149:10
    #7 0x5268e1 in main utilities/magick.c:180:10
    #8 0x7f06423b2b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
    #9 0x41b069 in _start (install/bin/magick+0x41b069)

0x611000000c80 is located 0 bytes to the right of 256-byte region [0x611000000b80,0x611000000c80)
allocated by thread T0 here:
    #0 0x4e5397 in __interceptor_malloc (install/bin/magick+0x4e5397)
    #1 0x7f064832b0b6 in AcquireMagickMemory MagickCore/memory.c:478:10
    #2 0x7f064832b11f in AcquireQuantumMemory MagickCore/memory.c:551:10
    #3 0x7f06484926e7 in AcquirePixelThreadSet MagickCore/statistic.c:182:33
    #4 0x7f0648490b21 in EvaluateImages MagickCore/statistic.c:499:19
    #5 0x7f0647aa55bf in CLIListOperatorImages MagickWand/operation.c:4084:22
    #6 0x7f0647aaf35e in CLIOption MagickWand/operation.c:5279:14
    #7 0x7f06478f0a99 in ProcessCommandOptions MagickWand/magick-cli.c:477:7
    #8 0x7f06478f1d0a in MagickImageCommand MagickWand/magick-cli.c:796:5
    #9 0x7f064793bba1 in MagickCommandGenesis MagickWand/mogrify.c:185:14
    #10 0x526f95 in MagickMain utilities/magick.c:149:10
    #11 0x5268e1 in main utilities/magick.c:180:10
    #12 0x7f06423b2b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)

SUMMARY: AddressSanitizer: heap-buffer-overflow MagickCore/statistic.c:559:43 in EvaluateImages

System Configuration

  • ImageMagick version:
    Version: ImageMagick 7.0.8-50 Q16 x86_64 2019-06-22 https://imagemagick.org

  • Environment (Operating system, version and so on):
    Description: Ubuntu 18.04.1 LTS
    Release: 18.04
    Codename: bionic

  • Additional information:
    CC=clang-7 CXX=clang++-7 ./configure --disable-openmp

urban-warrior pushed a commit to ImageMagick/ImageMagick6 that referenced this issue Jun 22, 2019
@urban-warrior
Copy link
Contributor

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.

@dlemstra dlemstra added the bug label Jun 22, 2019
@dlemstra dlemstra added this to the 7.0.8-50 milestone Jun 22, 2019
@nohmask
Copy link

nohmask commented Jul 8, 2019

This was assigned CVE-2019-13307.

@hlef
Copy link

hlef commented Aug 9, 2019

It is worth mentioning that ImageMagick/ImageMagick6@91e58d9 introduces a memory leak which is fixed later in ImageMagick/ImageMagick6@e6d26d4.

Also @urban-warrior, I'm not really sure to understand the logic behind ImageMagick/ImageMagick6@643921c.

What if images->columns is bigger than GetImageListLength(images) and bigger than all other next->columns? After ImageMagick/ImageMagick6@643921c, images->columns is basically ignored, right?

Then there might be a mismatch between the result of AcquireImageCanvas (image) and the result of AcquirePixelThreadSet (evaluate_pixels), which recreates this issue.

@urban-warrior
Copy link
Contributor

urban-warrior commented Aug 9, 2019

We check for the maximum columns in the image sequence. Next is aliased to images so that images->columns is checked suggesting image->columns is not ignored. Can you post a use case that we can reproduce that shows the memory corruption still persists?

@hlef
Copy link

hlef commented Aug 10, 2019

Thanks for your answer. Indeed, this patch does not recreate the issue, I was confused.

so, I suppose that GetImageListLength(images) is also considered here in order to avoid out of bounds write at https://github.com/ImageMagick/ImageMagick6/blob/master/magick/statistic.c#L585

@urban-warrior
Copy link
Contributor

Yes we use the thread set for two different use cases. Certainly as a convenience. Best coding practices would encourage the use of two different allocations, sized to fit, for each of these use cases.

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