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

memory exhaustion in ReadICONImage #466

Closed
jgj212 opened this issue May 3, 2017 · 4 comments
Closed

memory exhaustion in ReadICONImage #466

jgj212 opened this issue May 3, 2017 · 4 comments
Labels
bug

Comments

@jgj212
Copy link
Contributor

@jgj212 jgj212 commented May 3, 2017

ImageMagick 7.0.5-6

$magick identify $FILE

When identify icon file, imagemagick will allocate memory to store colormap in function ReadICONImage in coders\icon.c, line 449
//////////////////////////////////
if (AcquireImageColormap(image,image->colors,exception) ==
MagickFalse)
\\\\\\\\\\\\\\\\\\

image->colors can be controlled, as it is assigned as follow(line 431):
//////////////////////////////////
if ((icon_info.number_colors != 0) || (icon_info.bits_per_pixel <= 16U))
{
image->storage_class=PseudoClass;
image->colors=icon_info.number_colors; //can be controlled
if (image->colors == 0)
image->colors=one << icon_info.bits_per_pixel;
}
\\\\\\\\\\\\\\\\\\

icon_info.number_colors is diretly from icon file without checking( line 400)
//////////////////////////////////
icon_info.number_colors=ReadBlobLSBLong(image); //can be controlled by modify icon file
\\\\\\\\\\\\\\\\\\

So, modifying the number_colors can cause ImageMagick to allocate a anysize amount of memory, this may cause a memory exhaustion

Reproducer: https://github.com/jgj212/poc/blob/master/ImageMagick-7.0.5-6-colormap-memory-leak.ICON
Credit: ADLab of Venustech

@dlemstra

This comment has been minimized.

Copy link
Member

@dlemstra dlemstra commented May 3, 2017

Thanks for reporting this but this would not classify as a memory leak but as a possible memory exhaustion. You can prevent this with the policy.xml file. I do think we need to add an extra and check to prevent this though and change if (image->colors == 0) to if ((image->colors == 0) || (image->colors > 256))

@jgj212 jgj212 changed the title memory leak in ReadICONImage memory exhaustion in ReadICONImage May 3, 2017
@jgj212

This comment has been minimized.

Copy link
Contributor Author

@jgj212 jgj212 commented May 3, 2017

@dlemstra thank you, i changed the title

@dlemstra

This comment has been minimized.

Copy link
Member

@dlemstra dlemstra commented May 3, 2017

No problem and thanks for reporting this 👍. We will try to patch this later today. We are experiencing some issues with our synchronization to github so it might be a while before you will see the patch.

@dlemstra dlemstra added the bug label May 3, 2017
dlemstra added a commit that referenced this issue May 3, 2017
dlemstra added a commit that referenced this issue May 3, 2017
@carnil

This comment has been minimized.

Copy link

@carnil carnil commented May 4, 2017

This issue has been assigned CVE-2017-8765

@dlemstra dlemstra closed this May 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.