Bug #1208
Updated by Jörg Riesmeier 22 days ago
REPORTERS
---------
Cristhian Daniel Rivas Zúñiga and Sebastian Andres Muñoz Morera
Insituto Tecnológico de Costa Rica
---
SUMMARY
-------
A heap buffer overflow exists in XMLNode::parseFile() in ofstd/libsrc/ofxml.cc. When the function is called with a FIFO (named pipe) as input — which is a supported and documented use case via cda2dcm — the ftell() call returns -1 to signal an error. The code does not check for this error condition (it only checks for l == 0), causing malloc(3) to be called followed by fread() with a size_t-casted -1 value, resulting in an attempt to read up to SIZE_MAX bytes into a 3-byte heap buffer.
CWE: CWE-122 (Heap-based Buffer Overflow)
CVSS (estimated): 8.1 HIGH — AV:L/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H
---
AFFECTED VERSIONS
-----------------
DCMTK 3.6.7, 3.6.8, 3.6.9, 3.7.0, and current master branch.
The vulnerable lines (1962–1969) are identical across all versions checked.
---
ROOT CAUSE
----------
File: ofstd/libsrc/ofxml.cc
Function: XMLNode::parseFile()
Lines: ~1962–1969
The vulnerability chain:
1. fseek(f, 0, SEEK_END) succeeds on a FIFO but positions nothing meaningful.
2. int l = OFstatic_cast(int, ftell(f)) → ftell() returns -1 on a FIFO (POSIX: "undefined for non-seekable files").
3. if (!l) { ... return emptyXMLNode; } → This check only catches l == 0. It does NOT catch l == -1, so execution continues.
4. malloc(l + 4) = malloc(-1 + 4) = malloc(3) → A 3-byte heap buffer is allocated.
5. fread(buf, 1, l, f): the value -1 is implicitly cast to size_t, becoming SIZE_MAX (~18 exabytes). fread attempts to read SIZE_MAX bytes into a 3-byte buffer.
6. buf[l]=0; buf[l+1]=0; buf[l+2]=0; buf[l+3]=0; → Writes to buf[-1] through buf[2], corrupting adjacent heap memory.
---
CALL CHAIN
----------
cda2dcm (main)
→ OFstub_main() with --filetype-cda flag
→ DcmEncapsulatedDocument::parseArguments()
→ DcmEncapsulatedDocument::insertEncapsulatedDocument() [sets ftype_ = DT_cdaDocument]
→ DcmEncapsulatedDocument::formatSpecificProcessing()
→ DcmEncapsulatedDocument::getCDAData()
→ XMLNode::parseFile() ← VULNERABLE
---
PROOF OF CONCEPT
----------------
Prerequisites: DCMTK built with AddressSanitizer (-fsanitize=address).
Terminal 1 (reader):
export DCMDICTPATH=/path/to/dcmtk/dcmdata/data/dicom.dic
mkfifo /tmp/exploit.xml
./build-asan/bin/cda2dcm /tmp/exploit.xml out.dcm
Terminal 2 (writer, after ~1 second):
echo "<ClinicalDocument></ClinicalDocument>" > /tmp/exploit.xml
Expected output:
==ERROR: AddressSanitizer: heap-buffer-overflow
READ of size 18446744073709551615 at 0x...
The crash is 100% reproducible across all tested versions.
[Screenshot removed]
---
SUGGESTED FIX
-------------
In ofstd/libsrc/ofxml.cc, change line ~1964 from:
if (!l) { if (pResults) pResults->error=eXMLErrorEmpty; fclose(f); return emptyXMLNode; }
To:
if (l <= 0) { if (pResults) pResults->error=eXMLErrorEmpty; fclose(f); return emptyXMLNode; }
This single character change causes the function to correctly handle the ftell() error return value of -1.
---
[...]