From 31ff413f97839c61752875569862a94a6cce06dd Mon Sep 17 00:00:00 2001
From: Marco Eichelberg <dicom@offis.de>
Date: Thu, 28 Dec 2023 17:32:42 +0100
Subject: [PATCH] Fixed buffer overflow in decompression codecs.
Fixed the computation of the element size for the decompressed PixelData
element in all decompression codecs. In the case of datasets that cannot
be decoded to an unencapsulated transfer syntax, since the resulting
pixel data would be larger than 4 GByte, an overflow of the size
variable remained undetected and could lead to a segmentation fault due
to a buffer overrun.
This closes DCMTK issue #1099.
---
dcmdata/libsrc/dcrleccd.cc | 39 ++++++++++++++++++++++++++++++--------
dcmjpeg/libsrc/djcodecd.cc | 36 ++++++++++++++++++++++++++++++-----
dcmjpls/libsrc/djcodecd.cc | 17 ++++++++++++++++-
3 files changed, 78 insertions(+), 14 deletions(-)
diff --git a/dcmdata/libsrc/dcrleccd.cc b/dcmdata/libsrc/dcrleccd.cc
index ffdd14116..5ddc8f6be 100644
--- a/dcmdata/libsrc/dcrleccd.cc
+++ b/dcmdata/libsrc/dcrleccd.cc
@@ -1,6 +1,6 @@
/*
*
- * Copyright (C) 2002-2020, OFFIS e.V.
+ * Copyright (C) 2002-2024, OFFIS e.V.
* All rights reserved. See COPYRIGHT file for details.
*
* This software and supporting documentation were developed by
@@ -129,9 +129,25 @@ OFCondition DcmRLECodecDecoder::decode(
if (rledecoder.fail()) result = EC_MemoryExhausted; // RLE decoder failed to initialize
else
{
- const size_t frameSize = OFstatic_cast(size_t, imageBytesAllocated) * OFstatic_cast(size_t, imageRows)
- * OFstatic_cast(size_t, imageColumns) * OFstatic_cast(size_t, imageSamplesPerPixel);
- size_t totalSize = frameSize * imageFrames;
+ // compute size of uncompressed frame, in bytes
+ Uint32 frameSize = imageBytesAllocated * imageRows * imageColumns * imageSamplesPerPixel;
+
+ // check for overflow
+ if (imageRows != 0 && frameSize / imageRows != (imageBytesAllocated * imageColumns * imageSamplesPerPixel))
+ {
+ DCMDATA_WARN("Cannot decompress image because uncompressed representation would exceed maximum possible size of PixelData attribute.");
+ return EC_ElemLengthExceeds32BitField;
+ }
+
+ Uint32 totalSize = frameSize * imageFrames;
+
+ // check for overflow
+ if (totalSize == 0xFFFFFFFF || (frameSize != 0 && totalSize / frameSize != OFstatic_cast(Uint32, imageFrames)))
+ {
+ DCMDATA_WARN("Cannot decompress image because uncompressed representation would exceed maximum possible size of PixelData attribute.");
+ return EC_ElemLengthExceeds32BitField;
+ }
+
if (totalSize & 1) totalSize++; // align on 16-bit word boundary
Uint16 *imageData16 = NULL;
Sint32 currentFrame = 0;
@@ -139,7 +155,7 @@ OFCondition DcmRLECodecDecoder::decode(
Uint32 numberOfStripes = 0;
Uint32 fragmentLength = 0;
- result = uncompressedPixelData.createUint16Array(OFstatic_cast(Uint32, totalSize/sizeof(Uint16)), imageData16);
+ result = uncompressedPixelData.createUint16Array(totalSize/sizeof(Uint16), imageData16);
if (result.good())
{
Uint8 *imageData8 = OFreinterpret_cast(Uint8 *, imageData16);
@@ -463,7 +479,7 @@ OFCondition DcmRLECodecDecoder::decodeFrame(
Uint16 imageColumns = 0;
Sint32 imageFrames = 1;
Uint16 imageBitsAllocated = 0;
- Uint16 imageBytesAllocated = 0;
+ Uint32 imageBytesAllocated = 0;
Uint16 imagePlanarConfiguration = 0;
Uint32 rleHeader[16];
OFString photometricInterpretation;
@@ -476,7 +492,7 @@ OFCondition DcmRLECodecDecoder::decodeFrame(
if (result.good()) result = dataset->findAndGetOFString(DCM_PhotometricInterpretation, photometricInterpretation);
if (result.good())
{
- imageBytesAllocated = OFstatic_cast(Uint16, imageBitsAllocated / 8);
+ imageBytesAllocated = OFstatic_cast(Uint32, imageBitsAllocated / 8);
if ((imageBitsAllocated < 8)||(imageBitsAllocated % 8 != 0))
{
DCMDATA_ERROR("The RLE decoder only supports images where BitsAllocated is a multiple of 8.");
@@ -500,9 +516,16 @@ OFCondition DcmRLECodecDecoder::decodeFrame(
const size_t bytesPerStripe = OFstatic_cast(size_t, imageColumns) * OFstatic_cast(size_t, imageRows);
Uint32 numberOfStripes = 0;
Uint32 fragmentLength = 0;
- Uint32 frameSize = OFstatic_cast(Uint32, imageBytesAllocated) * OFstatic_cast(Uint32, imageRows)
+ Uint32 frameSize = imageBytesAllocated * OFstatic_cast(Uint32, imageRows)
* OFstatic_cast(Uint32, imageColumns) * OFstatic_cast(Uint32, imageSamplesPerPixel);
+ // check for overflow
+ if (imageRows != 0 && frameSize / imageRows != (imageBytesAllocated * imageColumns * imageSamplesPerPixel))
+ {
+ DCMDATA_WARN("Cannot decompress image because uncompressed representation would exceed maximum possible size of PixelData attribute.");
+ return EC_ElemLengthExceeds32BitField;
+ }
+
if (frameSize > bufSize) return EC_IllegalCall;
DcmRLEDecoder rledecoder(bytesPerStripe);
diff --git a/dcmjpeg/libsrc/djcodecd.cc b/dcmjpeg/libsrc/djcodecd.cc
index 364b75533..a93a8104b 100644
--- a/dcmjpeg/libsrc/djcodecd.cc
+++ b/dcmjpeg/libsrc/djcodecd.cc
@@ -1,6 +1,6 @@
/*
*
- * Copyright (C) 2001-2022, OFFIS e.V.
+ * Copyright (C) 2001-2024, OFFIS e.V.
* All rights reserved. See COPYRIGHT file for details.
*
* This software and supporting documentation were developed by
@@ -150,8 +150,25 @@ OFCondition DJCodecDecoder::decode(
if (jpeg == NULL) result = EC_MemoryExhausted;
else
{
- size_t frameSize = ((precision > 8) ? sizeof(Uint16) : sizeof(Uint8)) * imageRows * imageColumns * imageSamplesPerPixel;
- size_t totalSize = frameSize * imageFrames;
+ Uint32 imageBytesAllocated = (precision > 8) ? sizeof(Uint16) : sizeof(Uint8);
+ Uint32 frameSize = imageBytesAllocated * imageRows * imageColumns * imageSamplesPerPixel;
+
+ // check for overflow
+ if (imageRows != 0 && frameSize / imageRows != (imageBytesAllocated * imageColumns * imageSamplesPerPixel))
+ {
+ DCMJPEG_WARN("Cannot decompress image because uncompressed representation would exceed maximum possible size of PixelData attribute.");
+ return EC_ElemLengthExceeds32BitField;
+ }
+
+ Uint32 totalSize = frameSize * imageFrames;
+
+ // check for overflow
+ if (totalSize == 0xFFFFFFFF || (frameSize != 0 && totalSize / frameSize != OFstatic_cast(Uint32, imageFrames)))
+ {
+ DCMJPEG_WARN("Cannot decompress image because uncompressed representation would exceed maximum possible size of PixelData attribute.");
+ return EC_ElemLengthExceeds32BitField;
+ }
+
if (totalSize & 1) totalSize++; // align on 16-bit word boundary
Uint16 *imageData16 = NULL;
Sint32 currentFrame = 0;
@@ -166,7 +183,7 @@ OFCondition DJCodecDecoder::decode(
}
}
- result = uncompressedPixelData.createUint16Array(OFstatic_cast(Uint32, totalSize / sizeof(Uint16)), imageData16);
+ result = uncompressedPixelData.createUint16Array(totalSize / sizeof(Uint16), imageData16);
if (result.good())
{
Uint8 *imageData8 = OFreinterpret_cast(Uint8*, imageData16);
@@ -492,7 +509,16 @@ OFCondition DJCodecDecoder::decodeFrame(
if (precision == 0) result = EC_CannotChangeRepresentation; // something has gone wrong, bail out
else
{
- size_t frameSize = ((precision > 8) ? sizeof(Uint16) : sizeof(Uint8)) * imageRows * imageColumns * imageSamplesPerPixel;
+ Uint32 imageBytesAllocated = (precision > 8) ? sizeof(Uint16) : sizeof(Uint8);
+ Uint32 frameSize = imageBytesAllocated * imageRows * imageColumns * imageSamplesPerPixel;
+
+ // check for overflow
+ if (imageRows != 0 && frameSize / imageRows != (imageBytesAllocated * imageColumns * imageSamplesPerPixel))
+ {
+ DCMJPEG_WARN("Cannot decompress image because uncompressed representation would exceed maximum possible size of PixelData attribute.");
+ return EC_ElemLengthExceeds32BitField;
+ }
+
if (frameSize > bufSize) return EC_IllegalCall;
DJDecoder *jpeg = createDecoderInstance(fromParam, djcp, precision, isYBR);
diff --git a/dcmjpls/libsrc/djcodecd.cc b/dcmjpls/libsrc/djcodecd.cc
index e04a21e0d..45e6fec56 100644
--- a/dcmjpls/libsrc/djcodecd.cc
+++ b/dcmjpls/libsrc/djcodecd.cc
@@ -1,6 +1,6 @@
/*
*
- * Copyright (C) 2007-2022, OFFIS e.V.
+ * Copyright (C) 2007-2024, OFFIS e.V.
* All rights reserved. See COPYRIGHT file for details.
*
* This software and supporting documentation were developed by
@@ -143,8 +143,23 @@ OFCondition DJLSDecoderBase::decode(
// compute size of uncompressed frame, in bytes
Uint32 frameSize = bytesPerSample * imageRows * imageColumns * imageSamplesPerPixel;
+ // check for overflow
+ if (imageRows != 0 && frameSize / imageRows != (bytesPerSample * imageColumns * imageSamplesPerPixel))
+ {
+ DCMJPLS_WARN("Cannot decompress image because uncompressed representation would exceed maximum possible size of PixelData attribute.");
+ return EC_ElemLengthExceeds32BitField;
+ }
+
// compute size of pixel data attribute, in bytes
Uint32 totalSize = frameSize * imageFrames;
+
+ // check for overflow
+ if (totalSize == 0xFFFFFFFF || (frameSize != 0 && totalSize / frameSize != OFstatic_cast(Uint32, imageFrames)))
+ {
+ DCMJPLS_WARN("Cannot decompress image because uncompressed representation would exceed maximum possible size of PixelData attribute.");
+ return EC_ElemLengthExceeds32BitField;
+ }
+
if (totalSize & 1) totalSize++; // align on 16-bit word boundary
// assume we can cast the codec parameter to what we need
--
2.44.0