From d96c506197f34ef7214d3173377de58b9f72c032 Mon Sep 17 00:00:00 2001 From: Alan Karl Date: Tue, 25 Aug 2026 22:07:51 +0000 Subject: [PATCH 1/4] dicom-parse: check the source can supply an element before allocating parse_element_body() allocated length + 1 bytes straight from the element header and only then called dcm_require() to read the value, so a truncated or hostile file could make libdicom attempt an allocation of up to 4GB before discovering the data is not there. Reachable on the very first parse step. A 177 byte input whose File Meta Information declares an element of 0xFFFFFFFF bytes triggers a 4GB calloc() inside dcm_filehandle_get_file_meta(): ERROR: libFuzzer: out-of-memory (malloc(4294967296)) #8 dcm_calloc src/dicom.c:45 #9 parse_element_body src/dicom-parse.c:606 #10 dcm_parse_group src/dicom-parse.c:851 #11 dcm_filehandle_read_file_meta src/dicom-file.c:575 #12 dcm_filehandle_get_file_meta src/dicom-file.c:639 For a service that ingests DICOM from outside, a handful of small files is enough to exhaust memory; on a 32-bit or memory-capped host a single one aborts the process. Add dcm_remaining(), which reports the bytes left in a seekable source via the existing DcmIO seek method, and refuse an element that claims more than the source holds. Sources that cannot report a size (a custom DcmIO whose seek fails) return -1 and keep the previous behaviour, so streaming clients are unaffected. PixelData is routed to parse_pixeldata() before this point and is not affected either. The upstream check_dicom suite passes, and dcm-dump output is byte identical on all seven files in data/test_files. Found with the libFuzzer target added in this branch. --- src/dicom-parse.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/dicom-parse.c b/src/dicom-parse.c index 57bf21c..4b826a3 100644 --- a/src/dicom-parse.c +++ b/src/dicom-parse.c @@ -94,6 +94,31 @@ static bool dcm_seekcur(DcmParseState *state, int64_t offset, int64_t *position) } +/* Bytes left in a seekable source, or -1 if the source cannot report it. + * + * Used to reject element lengths that cannot possibly be satisfied before + * allocating a buffer for them. + */ +static int64_t dcm_remaining(DcmParseState *state) +{ + int64_t here = dcm_io_seek(NULL, state->io, 0, SEEK_CUR); + if (here < 0) { + return -1; + } + + int64_t end = dcm_io_seek(NULL, state->io, 0, SEEK_END); + if (end < 0) { + return -1; + } + + if (dcm_io_seek(NULL, state->io, here, SEEK_SET) < 0) { + return -1; + } + + return end - here; +} + + static bool dcm_is_eof(DcmParseState *state) { bool eof = true; @@ -603,6 +628,20 @@ static bool parse_element_body(DcmParseState *state, // read to a static char buffer, if possible if ((int64_t) length + 1 >= INPUT_BUFFER_SIZE) { + /* Don't allocate for a length the source cannot supply: a + * ~180 byte file declaring a 4GB element would otherwise + * make us try to allocate 4GB before the read fails. + */ + int64_t remaining = dcm_remaining(state); + if (remaining >= 0 && (int64_t) length > remaining) { + dcm_error_set(state->error, DCM_ERROR_CODE_PARSE, + "reading of data element failed", + "tag '%08x' declares %u bytes, " + "but only %zd remain", + tag, length, remaining); + return false; + } + value = value_free = DCM_MALLOC(state->error, (size_t) length + 1); if (value == NULL) { From bf4d00b76f4bcce5d2e013bb1209932e7b5ee9b0 Mon Sep 17 00:00:00 2001 From: Alan Karl Date: Wed, 26 Aug 2026 00:00:23 +0000 Subject: [PATCH 2/4] dicom-parse: bound encapsulated fragment growth by what the source holds dcm_parse_encapsulated_frame() reallocated the frame buffer to frame_length + fragment_length using the fragment length straight from the item header, checking only that the total stayed under 4GB. A 395 byte file declaring a fragment of 0xFF000008 therefore triggered: ERROR: libFuzzer: out-of-memory (malloc(4278190088)) #8 dcm_realloc src/dicom.c:58 #9 dcm_parse_encapsulated_frame src/dicom-parse.c:1131 #10 dcm_filehandle_read_frame src/dicom-file.c:1381 Same shape as the element-body allocation fixed earlier, in a path that fix did not cover. Reuse dcm_remaining() to reject a fragment longer than the source can supply. Sources that cannot report a size keep the previous behaviour. check_dicom passes; dcm-dump output is byte identical on all seven files in data/test_files. --- src/dicom-parse.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/dicom-parse.c b/src/dicom-parse.c index 4b826a3..159296a 100644 --- a/src/dicom-parse.c +++ b/src/dicom-parse.c @@ -1128,6 +1128,20 @@ char *dcm_parse_encapsulated_frame(DcmError **error, return NULL; } + /* Don't grow the buffer past what the source can actually supply: + * a few hundred bytes declaring a fragment of nearly 4GB would + * otherwise reallocate 4GB before the read fails. + */ + int64_t frame_remaining = dcm_remaining(&state); + if (frame_remaining >= 0 && (int64_t) fragment_length > frame_remaining) { + dcm_error_set(error, DCM_ERROR_CODE_PARSE, + "reading frame item failed", + "fragment declares %u bytes, but only %zd remain", + fragment_length, frame_remaining); + free(value); + return NULL; + } + char *new_value = (char *) dcm_realloc(error, value, frame_length + fragment_length); if (new_value == NULL) { From 90144fc6b5175aecd9378d6ca6aebddf0aeba31c Mon Sep 17 00:00:00 2001 From: Alan Karl Date: Wed, 26 Aug 2026 00:05:36 +0000 Subject: [PATCH 3/4] dicom-parse: bound PixelData item allocation by what the source holds Third instance of the same shape: parse_pixeldata_item() allocated item_length bytes from the item header before dcm_require() checked the data was present. ERROR: libFuzzer: out-of-memory (malloc(822083592)) #8 dcm_calloc src/dicom.c:45 #9 parse_pixeldata_item src/dicom-parse.c:473 #10 parse_pixeldata src/dicom-parse.c:560 Apply the dcm_remaining() guard here too. check_dicom passes; dcm-dump output is byte identical on all seven files in data/test_files. --- src/dicom-parse.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/dicom-parse.c b/src/dicom-parse.c index 159296a..cf9232c 100644 --- a/src/dicom-parse.c +++ b/src/dicom-parse.c @@ -470,6 +470,18 @@ static bool parse_pixeldata_item(DcmParseState *state, // read to our stack buffer, if possible if (item_length > INPUT_BUFFER_SIZE) { + /* As in parse_element_body(): don't allocate for an item length + * the source cannot supply. + */ + int64_t remaining = dcm_remaining(state); + if (remaining >= 0 && (int64_t) item_length > remaining) { + dcm_error_set(state->error, DCM_ERROR_CODE_PARSE, + "reading of PixelData item failed", + "item declares %u bytes, but only %zd remain", + item_length, remaining); + return false; + } + value = value_free = DCM_MALLOC(state->error, item_length); if (value_free == NULL) { return false; From 65be917fd208f8e12657cd5e555db02359288091 Mon Sep 17 00:00:00 2001 From: Alan Karl Date: Wed, 26 Aug 2026 02:05:36 +0000 Subject: [PATCH 4/4] dicom-parse: compute the frame size in 64 bits and range-check it dcm_parse_frame() computed *length = rows * columns * samples_per_pixel * (bits_allocated / 8); with four uint16_t operands, which promote to int, so the product overflows *signed* arithmetic -- undefined behaviour -- before it ever reaches the uint32_t it is stored in: dicom-parse.c:1078:41: runtime error: signed integer overflow: 1098696192 * 2 cannot be represented in type 'int' and the wrapped value is then handed to DCM_MALLOC: ERROR: libFuzzer: out-of-memory (malloc(2197392384)) Widening alone is not enough: 65535 x 65535 x 4 x 8 does not fit in 32 bits either, and *length is uint32_t in the API. Compute in uint64_t, reject a size that cannot be represented or that the source cannot supply, and only then allocate. Worth noting for callers: consumers that recompute the frame size from the rows/columns/samples/bits on DcmFrame would read past the buffer when the product wrapped small. check_dicom passes; dcm-dump output is byte identical on all seven files in data/test_files. --- src/dicom-parse.c | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/src/dicom-parse.c b/src/dicom-parse.c index cf9232c..4cff2cb 100644 --- a/src/dicom-parse.c +++ b/src/dicom-parse.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -1073,10 +1074,37 @@ char *dcm_parse_frame(DcmError **error, .big_endian = is_big_endian(), }; - *length = desc->rows * - desc->columns * - desc->samples_per_pixel * - (desc->bits_allocated / 8); + /* Compute in 64 bits: rows, columns and samples_per_pixel are all + * uint16_t, so they promote to int and the product overflows *signed* + * arithmetic (undefined behaviour) long before it overflows uint32_t. + * 65535 x 65535 x 4 x 8 does not fit in 32 bits either, so the result + * has to be range-checked, not just widened. + */ + uint64_t frame_length = (uint64_t) desc->rows * + (uint64_t) desc->columns * + (uint64_t) desc->samples_per_pixel * + (uint64_t) (desc->bits_allocated / 8); + if (frame_length == 0 || frame_length > UINT32_MAX) { + dcm_error_set(error, DCM_ERROR_CODE_PARSE, + "reading frame failed", + "implausible frame size from image description " + "(%u x %u, %u samples, %u bits)", + desc->rows, desc->columns, + desc->samples_per_pixel, desc->bits_allocated); + return NULL; + } + + /* And don't allocate for more than the source can supply. */ + int64_t frame_remaining = dcm_remaining(&state); + if (frame_remaining >= 0 && (int64_t) frame_length > frame_remaining) { + dcm_error_set(error, DCM_ERROR_CODE_PARSE, + "reading frame failed", + "frame needs %llu bytes, but only %zd remain", + (unsigned long long) frame_length, frame_remaining); + return NULL; + } + + *length = (uint32_t) frame_length; char *value = DCM_MALLOC(error, *length); if (value == NULL) {