Bug - Double free on `dcm_dataset_insert`

[voidz0r]

Introduction

Hi, I would like to report a security vulnerability about the libdicom library. It is possible to achieve a Double Free by putting two elements of the same TAG value in the data element header or data element body.

Attached you can find both examples. I tested the behavior by using the example on the README.md file.

#include <stdlib.h>
#include <dicom/dicom.h>

int main() {
    const char *file_path = "/path/to/file.dcm";
    DcmError *error = NULL;

    DcmFilehandle *filehandle = dcm_filehandle_create_from_file(&error, file_path);
    if (filehandle == NULL) {
        dcm_error_log(error);
        dcm_error_clear(&error);
        return 1;
    }

    const DcmDataSet *metadata =
        dcm_filehandle_get_metadata_subset(&error, filehandle);
    if (metadata == NULL) {
        dcm_error_log(error);
        dcm_error_clear(&error);
        dcm_filehandle_destroy(filehandle);
        return 1;
    }

    const char *num_frames;
    uint32_t tag = dcm_dict_tag_from_keyword("NumberOfFrames");
    DcmElement *element = dcm_dataset_get(&error, metadata, tag);
    if (element == NULL ||
        !dcm_element_get_value_string(&error, element, 0, &num_frames)) {
        dcm_error_log(error);
        dcm_error_clear(&error);
        dcm_filehandle_destroy(filehandle);
        return 1;
    }

    printf("NumerOfFrames == %s\n", num_frames);

    dcm_filehandle_destroy(filehandle);

    return 0;
}

And then execute the file with:

DCM_DEBUG=1 ./example <file.dcm>

Crash 1 - Data element headers

The first one is regarding the dcm_dataset_insert function that, whenever a duplicate tag is found, it free the memory and returns a boolean value of false. https://github.com/ImagingDataCommons/libdicom/blob/345eda55428b04182233615afeaa10ce10a0cbe1/src/dicom-data.c#L1432-L1440

After this, the caller parse_meta_element_create checks if the above calls returned false and try to free the pointer again, triggering a double free issue.

https://github.com/ImagingDataCommons/libdicom/blob/345eda55428b04182233615afeaa10ce10a0cbe1/src/dicom-file.c#L476-L482

This results in a crash since it tries to free a malformed memory address.

Debugging the application with gdb shows the following calls to dcm_element_destroy.

I've set a breakpoint on https://github.com/ImagingDataCommons/libdicom/blob/345eda55428b04182233615afeaa10ce10a0cbe1/src/dicom-data.c#L195

First hit of dcm_element_destroy

Second hit

The crash

Impact

Double free vulnerabilities could lead to remote code execution in some circumstances, in this case the workflow of the application and the logic does not allow to allocate/deallocate arbitrary chunks in a specific order, though the result is just a crash. Since the library is also used in the "openslide" project, the payloads can be used to crash some desktop DICOM readers such as QuPath, since it's based on openslide.

How to reproduce

I've made a simple DICOM builder with python at https://github.com/voidz0r/dicombuilder which can be used to build arbitrary tags into a file. I've included an "examples" directory and I'm attaching two sample files at the bottom.

Please note: this is valid for all the data elements, including sequences. The affected function is the same.

Solution

The solution, depending on how the application has been built, would be to remove the additional call to dcm_element_destroy on the dcm_dataset_insert function and leave the one in parse_meta_element_create.

Please let me know if I can help in some way.

proof_of_concept.zip

[jcupitt]

Hi @voidz0r, thank you very much for the detailed report.

This was actually reported and fixed a few weeks ago:

https://github.com/ImagingDataCommons/libdicom/pull/81

I need to get around to merging it. There's another PR in preparation to go in, then I plan to make libdicom v1.1.

[voidz0r]

This was quick! Thanks for the feedback anyway