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

Array Index Out of Bounds and Potential Buffer-Overflow due to user-controlled value being used as Array Index #705

Closed
kirit1193 opened this issue Aug 29, 2017 · 2 comments
Labels

Comments

@kirit1193
Copy link

An issue affects ImageMagick/coders/thumbnail.c in the WriteTHUMBNAILImage function, where an externally controllable value is being used as an index in the process of generating a thumbnail image. This can lead to potential Buffer Overflow. The vulnerable code is:

offset=(ssize_t) StringToLong(property);
property=GetImageProperty(image,"exif:JPEGInterchangeFormatLength",exception);
if (property == (const char *) NULL)
  ThrowWriterException(CoderError,"ImageDoesNotHaveAThumbnail");
length=(size_t) StringToLong(property);
(void) ResetMagickMemory(magick,0,sizeof(magick));
for (i=0; i < (ssize_t) length; i++)
  {
  magick[0]=magick[1];
  magick[1]=magick[2];
  magick[2]=GetStringInfoDatum(profile)[offset+i];
  if (memcmp(magick,"\377\330\377",3) == 0)
    break;
  }
thumbnail_image=BlobToImage(image_info,GetStringInfoDatum(profile)+offset+i-2,length,exception);

Here the offset variable gets its value from StringToLong being run on an input image.

This value of offset is not being sanitized before it is being used as an array index in:

magick[2]=GetStringInfoDatum(profile)[offset+i];

It is also being used to generate the thumbnail_image by calling the BlobToImage function, where the value of const void *blob depends on the value of offset.

Validation is being performed on whether *blob is NULL using:

if ((blob == (const void *) NULL) \|\| (length == 0))
  {
  (void) ThrowMagickException(exception,GetMagickModule(),BlobError,
  "ZeroLengthBlobNotPermitted","`%s'",image_info->filename);
  return((Image *) NULL);
  }

However, there are no checks on whether it is too large.

@mikayla-grace
Copy link

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.

@nohmask
Copy link

nohmask commented Sep 8, 2017

This was assigned CVE-2017-13769.

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

4 participants