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

A null pointer dereferene might happend in ImageMagick/MagickCore/property.c #1225

Closed
boo0m opened this issue Jul 23, 2018 · 11 comments
Closed
Labels

Comments

@boo0m
Copy link

boo0m commented Jul 23, 2018

Prerequisites

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

Description

In ImageMagick/MagickCore/property.c:2825, the code has a assert to see whether image is null image != (Image *) NULL || image_info != (ImageInfo *) NULL, but the condition still to be true if image_info is not null. And then in line:2828 comparing "image" to null implies that "image" might be null. The code don't do anything and it might to be a null pointer dereferene in switch case code like  string=CommandOptionToMnemonic(MagickInterlaceOptions,(ssize_t) image->interlace)

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

@dlemstra dlemstra added the bug label Jul 24, 2018
@nohmask
Copy link

nohmask commented Sep 3, 2018

This was assigned CVE-2018-16329.

@bastien-roucaries
Copy link

Do you have the im6 commit ?

@bastien-roucaries
Copy link

@urban-warrior @dlemstra Could we have the im6 commit ?

@dlemstra
Copy link
Member

There is no IM6 commit for this.

@bastien-roucaries
Copy link

@dlemstra So it does not affect im6 ?

@bastien-roucaries
Copy link

Does it affect IM6 ?

@urban-warrior
Copy link
Contributor

IMv7 handles property and command-line parsing differently than IMv6. This flaw was in the new property handler in IMv7 only. It does not affect IMv6.

@ret2libc
Copy link

ret2libc commented Apr 1, 2020

@urban-warrior Hi, I know you already said IM6 was not affected by this issue, however some time has passed and I see the IM6 code is very similar to the vulnerable one (https://github.com/ImageMagick/ImageMagick6/blob/master/magick/property.c#L3014). Could you confirm IM6 is still not affected?

urban-warrior pushed a commit to ImageMagick/ImageMagick6 that referenced this issue Apr 1, 2020
@urban-warrior
Copy link
Contributor

urban-warrior commented Apr 1, 2020

Note the assertion in IM6/property.c/GetMagickPropertyLetter(). It checks to ensure that the image is not null. In IMv7, GetMagickPropertyLetter() accepts NULL Image and ImageInfo structures. However, for IMv6, we need to check for a NULL ImageInfo structure. We'll add a patch. Thanks.

@ret2libc
Copy link

ret2libc commented Apr 2, 2020

@urban-warrior thanks for the reply. I was looking at GetMagickProperty indeed and I did not see any assert there, so I guess the same would should be done for that function as well.

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

6 participants