# Heap buffer overflow in itkImportImageContainer
**URL:** https://discourse.itk.org/t/heap-buffer-overflow-in-itkimportimagecontainer/2343
**Category:** Community
**Tags:** python, dicom, itk-releases, simpleitk
**Created:** [October 22, 2019, 6:33pm UTC](https://discourse.itk.org/t/heap-buffer-overflow-in-itkimportimagecontainer/2343 "2019-10-22T18:33:49Z")
**Posts on this page:** 8
**Page:** 1
### Author:  [@msh\_smlv](https://discourse.itk.org/u/msh_smlv)
#### Post date: [October 22, 2019, 6:33pm UTC](https://discourse.itk.org/t/heap-buffer-overflow-in-itkimportimagecontainer/2343/1 "2019-10-22T18:33:49Z")
Hello!
During an internal security assessment of the medical ML pipeline based on Simple-itk we found heap-buffer-overflow in DicomReader.
In the attached file you can find an example of a file that triggers the exception.
[example.tar.gz](https://discourse.itk.org/uploads/short-url/eoU6avaC6QD9TCdq7077IvCJg4S.gz) (269.5 KB)
---
### Author:  [@mihail.isakov](https://discourse.itk.org/u/mihail.isakov)
#### Post date: [October 22, 2019, 8:06pm UTC](https://discourse.itk.org/t/heap-buffer-overflow-in-itkimportimagecontainer/2343/2 "2019-10-22T20:06:06Z")
The image has
(0028,1053) Rescale Slope **-1024** and no (0028,1052) Rescale Intercept attribute, is it wrong, should be
(0028,1053) Rescale Slope **1**
(0028,1052) Rescale Intercept **-1024**
**Edit** :
and, BTW, Pixel Padding Value 65536 is wrong too (left as is)
**Edit 2** :
There is Pixel Representation **1** (2’s complement, so -1024 may be not required at all or it is wrong too), wait a minute…
**Edit 3** :
Sorry, there are too many things broken to speak about, this [version](https://drive.google.com/file/d/1bOpiZ_1oaq0sWoyAt2jSvw5CgqMkN5Mr/view?usp=sharing) will open so far HU consistent, i hope
---
### Author:  [@blowekamp](https://discourse.itk.org/u/blowekamp)
#### Post date: [October 23, 2019, 2:22pm UTC](https://discourse.itk.org/t/heap-buffer-overflow-in-itkimportimagecontainer/2343/3 "2019-10-23T14:22:37Z")
@mathieu.malaterre Any insight as to what might be that problem here?
---
### Author:  [@blowekamp](https://discourse.itk.org/u/blowekamp)
#### Post date: [October 23, 2019, 2:33pm UTC](https://discourse.itk.org/t/heap-buffer-overflow-in-itkimportimagecontainer/2343/4 "2019-10-23T14:33:51Z")
IMHO we should add a check in itkGDCMImageIO when copying GDCM buffer to ITK and visa versa that the size of the buffers are exactly the same. Yes, this is an intentional not compliant image trying to exploit the code, but we should add some checks of assumptions.
---
### Author:  [@mathieu.malaterre](https://discourse.itk.org/u/mathieu.malaterre)
#### Post date: [October 23, 2019, 3:13pm UTC](https://discourse.itk.org/t/heap-buffer-overflow-in-itkimportimagecontainer/2343/5 "2019-10-23T15:13:37Z")
I confirm the issue is deep down in GDCM. I’ll try to provide a complete analysis ASAP, and possible a fix. Thanks for the sample dataset.
---
### Author:  [@mathieu.malaterre](https://discourse.itk.org/u/mathieu.malaterre)
#### Post date: [October 24, 2019, 6:10am UTC](https://discourse.itk.org/t/heap-buffer-overflow-in-itkimportimagecontainer/2343/6 "2019-10-24T06:10:36Z")
@mihail.isakov I like your analysis ! Here is mine. The following attribute does not make much sense indeed:
`(0028,1053) DS [-1024] # 6,1 Rescale Slope`
If you look carefully a second attribute is actually send in the DataSet:
`(0028,1053) DS [1] # 6,1 Rescale Slope`
Since both GDCM & DCMTK uses the same convention: preserve the first one, the second one is actually never seen. So indeed the bug is indeed coming from the vendor machine, which send a Rescale Slope attribute in place of a Rescale Intercept.
As a side note, in this particular DataSet, the PixelData is already in the correct unit. So I believe the correct Attributes are:
```
(0028,1052) DS [0] # 6,1 Rescale Intercept
(0028,1053) DS [1] # 6,1 Rescale Slope
```
For reference here is a truncated output of `dcdump` (dicom3tools):
```
[...]
|(0x0028,0x1050) DS Window Center | VR= VL=<0x0004> <-400> |
|(0x0028,0x1051) DS Window Width | VR= VL=<0x0004> <1500> |
|(0x0028,0x1053) DS Rescale Slope | VR= VL=<0x0002> <1 > |
|(0x0028,0x1053) DS Rescale Slope | VR= VL=<0x0006> <-1024 > |
|(0x7fe0,0x0010) OX Pixel Data | VR= VL=<0x80000>|
[...]
```
(I do not know why the attributes are being printed in opposite order of the file order)
In any cases, GDCM codebase should not trigger a heap buffer overflow. I’ll add a guard against this case.
---
### Author:  [@mihail.isakov](https://discourse.itk.org/u/mihail.isakov)
#### Post date: [October 24, 2019, 1:36pm UTC](https://discourse.itk.org/t/heap-buffer-overflow-in-itkimportimagecontainer/2343/7 "2019-10-24T13:36:06Z")
(0x0028,0x1053) DS Rescale Slope | VR= VL=\<0x0002\> \<1 \>
(0x0028,0x1053) DS Rescale Slope | VR= VL=\<0x0006\> \<-1024 \>
Great! BTW, i wish the same behavior in my GUI metadata viewer too.
Tried to test with
```
-typedef std::set DataElementSet;
+typedef std::multiset DataElementSet;
```
seems to work (it is not a request, just playing and testing). May be a warning in release mode were great, it happens [here](https://github.com/InsightSoftwareConsortium/ITK/blob/a48375cc300b23034099526558d61678fdf99e05/Modules/ThirdParty/GDCM/src/gdcm/Source/DataStructureAndEncodingDefinition/gdcmDataSet.h#L289).
> I do not know why the attributes are being printed in opposite order of the file order
Some kind of sorting, probably, multimap or whatever sorting things around, just guessing.
Thank you.
---
### Author:  [@dzenanz](https://discourse.itk.org/u/dzenanz)
#### Post date: [October 24, 2019, 3:22pm UTC](https://discourse.itk.org/t/heap-buffer-overflow-in-itkimportimagecontainer/2343/8 "2019-10-24T15:22:42Z")
A fix was commit via this PR:
>