CVE-2024-28130
An incorrect type conversion vulnerability exists in the DVPSSoftcopyVOI_PList::createFromImage functionality of OFFIS DCMTK 3.6.8. A specially crafted malformed file can lead to arbitrary code execution. An attacker can provide a malicious file to trigger this vulnerability.
The versions below were either tested or verified to be vulnerable by Talos or confirmed to be vulnerable by the vendor.
OFFIS DCMTK 3.6.8
DCMTK - https://dicom.offis.de/dcmtk.php.en
7.5 - CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-704 - Incorrect Type Conversion or Cast
DCMTK is a collection of libraries and applications implementing large parts the DICOM standard.
It includes software …
for examining, constructing and converting DICOM image files
handling storage media
sending and receiving images over a network connection
as well as demonstrative image storage and worklist servers
DCMTK is is written in a mixture of ANSI C and C++. It comes in complete source code and is made available as open source software.
DCMTK has been used at numerous DICOM demonstrations to provide central, vendor-independent image storage and worklist servers (CTNs - Central Test Nodes).
It is used by hospitals and companies all over the world for a wide variety of purposes ranging from being a tool for product testing to being a building block for research projects, prototypes and commercial products.
A specially-crafted DICOM file can lead to an incorrect cast type in DVPSSoftcopyVOI_PList::createFromImage
, due to a missing check type of the record processed.
Below some extract of the function and the crash is happening in the following function DcmByteString::putOFStringAtPos
LINE477:
LINE471 OFCondition DcmByteString::putOFStringAtPos(const OFString& stringVal,
LINE472 const unsigned long pos)
LINE473 {
LINE474 OFCondition result;
LINE475 // Get old value
LINE476 OFString str;
LINE477 result = getOFStringArray( str );
LINE478 if (result.good())
LINE479 {
LINE480 size_t currentVM = getNumberOfValues();
LINE481 // Trivial case: No values are set and new value should go to first position
LINE482 if ( (currentVM == 0) && (pos == 0))
LINE483 return putOFStringArray(stringVal);
LINE484
LINE485 // 1st case: Insert at the end
LINE486 // If we insert at a position that does not yet exist, append missing number of components by
LINE487 // adding the corresponding number of backspace chars, append new float value and return.
LINE488 size_t futureVM = pos + 1;
LINE489 if (futureVM > currentVM)
LINE490 {
LINE491 str = str.append(currentVM == 0 ? futureVM - currentVM - 1 : futureVM - currentVM, '\\');
LINE492 str = str.append(stringVal);
LINE493 return putOFStringArray(str);
LINE494 [...]
In order to understand why it’s crashing, we have to get throught the call stack :
#0 DcmByteString::putOFStringAtPos (this=0x72, stringVal=..., pos=<optimized out>) at /home/manu/dcmtk/dcmdata/libsrc/dcbytstr.cc:477
#1 0x000055555589bed4 in DVPSSoftcopyVOI_PList::createFromImage (this=<optimized out>, dset=..., allReferences=..., sopclassUID=<optimized out>, instanceUID=<optimized out>, voiActivation=<optimized out>) at /home/manu/dcmtk/dcmpstat/libsrc/dvpssvl.cc:255
We can observe from the call stack something is wrong in argument, look at this
value equal to 0x72
, something is really wrong here.
Let see as disassembly corresponding function DcmByteString::putOFStringAtPos
below :
.text:0000000000485C10 ; OFCondition *__fastcall DcmByteString::putOFStringAtPos(OFCondition *__return_ptr retstr, DcmByteString *const this, const OFString *stringVal, size_t pos)
.text:0000000000485C10 public _ZN13DcmByteString16putOFStringAtPosERK8OFStringm
.text:0000000000485C10 _ZN13DcmByteString16putOFStringAtPosERK8OFStringm proc near
.text:0000000000485C10 ; DATA XREF: .data.rel.ro:00000000006E9840↓o
.text:0000000000485C10 ; .data.rel.ro:00000000006EC068↓o ...
.text:0000000000485C10
.text:0000000000485C10 pos = qword ptr -0E0h
.text:0000000000485C10 stringVal = qword ptr -0D8h
.text:0000000000485C10 this = qword ptr -0D0h
.text:0000000000485C10 var_C8 = qword ptr -0C8h
.text:0000000000485C10 rightPos = qword ptr -0B8h
.text:0000000000485C10 leftPos = qword ptr -0B0h
.text:0000000000485C10 vmPos = qword ptr -0A8h
.text:0000000000485C10 currentVM = qword ptr -0A0h
.text:0000000000485C10 futureVM = qword ptr -98h
.text:0000000000485C10 result = OFCondition ptr -90h
.text:0000000000485C10 str = OFString ptr -70h
.text:0000000000485C10 arg = OFCondition ptr -50h
.text:0000000000485C10 var_30 = OFCondition ptr -30h
.text:0000000000485C10 var_18 = qword ptr -18h
.text:0000000000485C10 var_8 = qword ptr -8
.text:0000000000485C10
.text:0000000000485C10 ; __unwind { // __gxx_personality_v0
.text:0000000000485C10 endbr64
.text:0000000000485C14 push rbp
.text:0000000000485C15 mov rbp, rsp
.text:0000000000485C18 push rbx
.text:0000000000485C19 sub rsp, 0D8h
.text:0000000000485C20 mov [rbp+var_C8], rdi
.text:0000000000485C27 mov [rbp-0D0h], rsi
.text:0000000000485C2E mov [rbp+stringVal], rdx
.text:0000000000485C35 mov [rbp+pos], rcx
.text:0000000000485C3C mov rax, fs:28h
.text:0000000000485C45 mov [rbp+var_18], rax
.text:0000000000485C49 xor eax, eax
.text:0000000000485C4B lea rax, [rbp+result]
.text:0000000000485C52 lea rdx, EC_Normal
.text:0000000000485C59 mov rsi, rdx ; aConst
.text:0000000000485C5C mov rdi, rax ; this
.text:0000000000485C5F call _ZN11OFConditionC2ERK16OFConditionConst ; OFCondition::OFCondition(OFConditionConst const&)
.text:0000000000485C64 lea rax, [rbp+str]
.text:0000000000485C68 mov rdi, rax ; this
.text:0000000000485C6B ; try {
.text:0000000000485C6B call _ZN8OFStringC2Ev ; OFString::OFString(void)
.text:0000000000485C6B ; } // starts at 485C6B
.text:0000000000485C70 mov rax, [rbp+this]
.text:0000000000485C77 mov rax, [rax]
.text:0000000000485C7A add rax, 190h
.text:0000000000485C80 mov r8, [rax]
.text:0000000000485C83 lea rax, [rbp+arg]
.text:0000000000485C87 lea rdx, [rbp+str]
.text:0000000000485C8B mov rsi, [rbp+this]
.text:0000000000485C92 mov ecx, 1
.text:0000000000485C97 mov rdi, rax
.text:0000000000485C9A ; try {
.text:0000000000485C9A call r8
The crash is corresponding to the dereference pointer through instruction at ` .text:0000000000485C80 mov r8, [rax] .
rax is function pointer computed at
0000000000485C7A by an offset of
190h from the vtable computed at
0000000000485C77 , by object reference as
this at
0000000000485C70, The
this is a
DcmByteString as identified by the function
DcmByteString::putOFStringAtPos` .
Getting through the call stack, the caller is DVPSSoftcopyVOI_PList::createFromImage
The source code function DVPSSoftcopyVOI_PList::createFromImage
at line 255 is corresponding to :
LINE224 OFCondition DVPSSoftcopyVOI_PList::createFromImage(
LINE225 DcmItem &dset,
LINE226 DVPSReferencedSeries_PList& allReferences,
LINE227 const char *sopclassUID,
LINE228 const char *instanceUID,
LINE229 DVPSVOIActivation voiActivation)
LINE230 {
LINE231 if (voiActivation == DVPSV_ignoreVOI) return EC_Normal;
LINE232
LINE233 OFCondition result = EC_Normal;
LINE234 DcmStack stack;
LINE235 DcmSequenceOfItems *seq;
LINE236 DcmItem *item;
LINE237 DcmUnsignedShort voiLUTDescriptor(DCM_LUTDescriptor);
LINE238 DcmLongString voiLUTExplanation(DCM_LUTExplanation);
LINE239 DcmUnsignedShort voiLUTData(DCM_LUTData);
LINE240 DcmDecimalString windowCenter(DCM_WindowCenter);
LINE241 DcmDecimalString windowWidth(DCM_WindowWidth);
LINE242 DcmLongString windowCenterWidthExplanation(DCM_WindowCenterWidthExplanation);
LINE243
LINE244 READ_FROM_DATASET(DcmDecimalString, EVR_DS, windowCenter)
LINE245 READ_FROM_DATASET(DcmDecimalString, EVR_DS, windowWidth)
LINE246 READ_FROM_DATASET(DcmLongString, EVR_LO, windowCenterWidthExplanation)
LINE247
LINE248 /* read VOI LUT Sequence */
LINE249 if (result==EC_Normal)
LINE250 {
LINE251 stack.clear();
LINE252 if (EC_Normal == dset.search(DCM_VOILUTSequence, stack, ESM_fromHere, OFFalse))
LINE253 {
LINE254 seq=(DcmSequenceOfItems *)stack.top();
LINE255 if (seq->card() > 0)
LINE256 {
and the instruction responsible for to DcmByteString::putOFStringAtPos
is done through the instruction seq->card()
call At LINE254. We can noticeseq
is casted into a (DcmSequenceOfItems *)
Going throught the definition of DcmSequenceOfItems
we can read :
/** class representing a DICOM Sequence of Items (SQ).
* This class is derived from class DcmElement (and not from DcmObject) despite the fact
* that sequences have no value field as such, they maintain a list of items. However,
* all APIs in class DcmItem and class DcmDataset accept DcmElements.
* This is ugly and causes some DcmElement API methods to be useless with DcmSequence.
*/
class DCMTK_DCMDATA_EXPORT DcmSequenceOfItems : public DcmElement
which is containing a virtual function named card
:
/** get cardinality of this sequence
* @return number of items in this sequence
*/
virtual unsigned long card() const;
Now what if the return object from stack.top()
at LINE254 is not at all corresponding to a DcmSequenceOfItems *
but instead for example an DcmByteString
object., then the this
pointer will correspond to another object. The issue is happening because the call to search
function at LIN252
is not checking the type of object to be returned making the cast invalid and lead to arbitrary code execution.
received signal SIGSEGV, Segmentation fault.
DcmByteString::putOFStringAtPos (this=0x72, stringVal=..., pos=<optimized out>) at /home/manu/dcmtk/dcmdata/libsrc/dcbytstr.cc:477
#0 DcmByteString::putOFStringAtPos (this=0x72, stringVal=..., pos=<optimized out>) at /home/manu/dcmtk/dcmdata/libsrc/dcbytstr.cc:477
#1 0x000055555589bed4 in DVPSSoftcopyVOI_PList::createFromImage (this=<optimized out>, dset=..., allReferences=..., sopclassUID=<optimized out>, instanceUID=<optimized out>, voiActivation=<optimized out>) at /home/manu/dcmtk/dcmpstat/libsrc/dvpssvl.cc:255
#2 0x000055555573b414 in DcmPresentationState::createFromImage (this=this@entry=0x555556b13350, dset=..., overlayActivation=overlayActivation@entry=DVPSO_copyOverlays, voiActivation=voiActivation@entry=DVPSV_preferVOILUT, curveActivation=curveActivation@entry=true, shutterActivation=<optimized out>, presentationActivation=<optimized out>, layering=<optimized out>, aetitle=<optimized out>, filesetID=<optimized out>, filesetUID=<optimized out>) at /home/manu/dcmtk/dcmpstat/libsrc/dcmpstat.cc:1093
#3 0x00005555558aa42a in DVPresentationState::createFromImage (this=this@entry=0x555556b13350, dset=..., overlayActivation=overlayActivation@entry=DVPSO_copyOverlays, voiActivation=voiActivation@entry=DVPSV_preferVOILUT, curveActivation=curveActivation@entry=true, shutterActivation=true, presentationActivation=true, layering=DVPSG_twoLayers, aetitle=0x0, filesetID=0x0, filesetUID=0x0) at /home/manu/dcmtk/dcmpstat/libsrc/dvpstat.cc:2156
#4 0x000055555574d1e9 in DVInterface::loadImage (this=this@entry=0x7fffffffbd10, imgName=<optimized out>) at /home/manu/dcmtk/dcmpstat/libsrc/dviface.cc:311
#5 0x00005555556d32b5 in main (argc=<optimized out>, argv=<optimized out>) at /home/manu/dcmtk/dcmpstat/apps/dcmp2pgm.cc:513
rax 0x555556b157a0 93825015044000
rbx 0xffffffffffffffe0 -32
rcx 0x1 1
rdx 0x7fffffffa790 140737488332688
rsi 0x72 114
rdi 0x7fffffffa7b0 140737488332720
rbp 0x5555567b2040 <__afl_area_initial>
rsp 0x7fffffffa750 140737488332624
r8 0x3a 58
r9 0x5b 91
r10 0xba 186
r11 0x5555567b9b21 93825011522337
r12 0x72 114
r13 0x7fffffffa790 140737488332688
r14 0x555556b24dc0 93825015107008
r15 0x5555567b2040 93825011490880
rip 0x555556300d7e <DcmByteString::putOFStringAtPos(OFString const&, unsigned long)+158>
=> 0x555556300d7e <_ZN13DcmByteString16putOFStringAtPosERK8OFStringm+158>: mov r8,QWORD PTR [r12]
0x555556300d82 <_ZN13DcmByteString16putOFStringAtPosERK8OFStringm+162>: call QWORD PTR [r8+0x190]
The vendor has provided an update in their Git repository.
2024-03-14 - Initial Vendor Contact
2024-04-08 - Vendor Disclosure
2024-04-22 - Vendor Patch Release
2024-04-23 - Public Release
Discovered by Emmanuel Tacheau of Cisco Talos.