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

Missing check for length in function ReadDCMImage of coders/dcm.c and function ReadPICTImage of coders/pict.c #1269

Closed
YangY-Xiao opened this issue Aug 27, 2018 · 2 comments
Labels

Comments

@YangY-Xiao
Copy link

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

There are two missing check for variable length.

3853           tag=((unsigned int) ReadBlobLSBShort(image) << 16) |
3854             ReadBlobLSBShort(image);
3855           length=(size_t) ReadBlobLSBLong(image);
3856           if (tag == 0xFFFEE0DD)
3857             break; /* sequence delimiter tag */
3858           if (tag != 0xFFFEE000)
3859             {
3860               read_info=DestroyImageInfo(read_info);
3861               ThrowDCMException(CorruptImageError,"ImproperImageHeader");
3862             }
3863           file=(FILE *) NULL;
3864           unique_file=AcquireUniqueFileResource(filename);
3865           if (unique_file != -1)
3866             file=fdopen(unique_file,"wb");
3867           if (file == (FILE *) NULL)
3868             {
3869               (void) RelinquishUniqueFileResource(filename);
3870               ThrowFileException(exception,FileOpenError,
3871                 "UnableToCreateTemporaryFile",filename);
3872               break;
3873             }

(coders/dcm.c)

1032             length=ReadBlobMSBShort(image);
1033             if (ReadRectangle(image,&frame) == MagickFalse)
1034               ThrowPICTException(CorruptImageError,"ImproperImageHeader");
1035             if (ReadPixmap(image,&pixmap) == MagickFalse)
1036               ThrowPICTException(CorruptImageError,"ImproperImageHeader");
...
1043             length=ReadBlobMSBShort(image);
1044             for (i=0; i <= (ssize_t) length; i++)
1045               (void) ReadBlobMSBLong(image);
1046             width=(size_t) (frame.bottom-frame.top);
1047             height=(size_t) (frame.right-frame.left);
...
1098           case 0x77:
1099           {
1100             /*
1101               Skip polygon or region.
1102             */
1103             length=ReadBlobMSBShort(image);
1104             for (i=0; i < (ssize_t) (length-2); i++)
1105               if (ReadBlobByte(image) == EOF)
1106                 break;
1107             break;
1108           }
...
1220             if ((code == 0x91) || (code == 0x99) || (code == 0x9b))
1221               {
1222                 /*
1223                   Skip region.
1224                 */
1225                 length=ReadBlobMSBShort(image);
1226                 for (i=0; i < (ssize_t) (length-2); i++)
1227                   if (ReadBlobByte(image) == EOF)
1228                     break;
1229               }

(coders/pict.c)

In my opinion, we should check whether length is bigger than GetBlobSize(image) or not. If condition length > GetBlobSize(image) satisfies, we should throw exception like ThrowDCMException(CorruptImageError,"InsufficientImageDataInFile").

  • ImageMagick version: latest version
urban-warrior pushed a commit to ImageMagick/ImageMagick6 that referenced this issue Aug 27, 2018
urban-warrior pushed a commit to ImageMagick/ImageMagick6 that referenced this issue Aug 27, 2018
@urban-warrior
Copy link
Member

We applied your patch to the ImageMagick trunk. Thanks.

@nohmask
Copy link

nohmask commented Sep 7, 2018

This was assigned CVE-2018-16644.

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