-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[dcmtk] Backport fixes for CVEs and data corruption bug (#26008)
* Backport vulnerability fix for CVE-2024-34508 and CVE-2024-34509 * Backport disabling iconv passthrough by default This fixes https://support.dcmtk.org/redmine/issues/1143 * Needs a recipe change to export the new patches --------- Co-authored-by: Abril Rincón Blanco <[email protected]> Co-authored-by: Abril Rincón Blanco <[email protected]>
- Loading branch information
3 people
authored
Nov 25, 2024
1 parent
1a2e1ea
commit 3acf5bc
Showing
5 changed files
with
171 additions
and
4 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
60 changes: 60 additions & 0 deletions
60
recipes/dcmtk/all/patches/3.6.7-0004-fix-two-segmentation-faults.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| --- a/dcmdata/libsrc/dcelem.cc | ||
| +++ b/dcmdata/libsrc/dcelem.cc | ||
| @@ -717,6 +717,13 @@ OFCondition DcmElement::loadValue(DcmInputStream *inStream) | ||
| if (isStreamNew) | ||
| delete readStream; | ||
| } | ||
| + else | ||
| + { | ||
| + errorFlag = EC_InvalidStream; // incomplete dataset read from stream | ||
| + DCMDATA_ERROR("DcmElement: " << getTagName() << " " << getTag() | ||
| + << " larger (" << getLengthField() << ") than remaining bytes (" | ||
| + << getTransferredBytes() << ") in file, premature end of stream"); | ||
| + } | ||
| } | ||
| /* return result value */ | ||
| return errorFlag; | ||
| diff --git a/dcmnet/libsrc/dimcmd.cc b/dcmnet/libsrc/dimcmd.cc | ||
| index 6dca39546..ffd225f4b 100644 | ||
| --- a/dcmnet/libsrc/dimcmd.cc | ||
| +++ b/dcmnet/libsrc/dimcmd.cc | ||
| @@ -205,22 +205,25 @@ getString(DcmDataset *obj, DcmTagKey t, char *s, int maxlen, OFBool *spacePadded | ||
| return parseErrorWithMsg("dimcmd:getString: string too small", t); | ||
| } else { | ||
| ec = elem->getString(aString); | ||
| - strncpy(s, aString, maxlen); | ||
| - if (spacePadded) | ||
| + if (ec.good()) | ||
| { | ||
| - /* before we remove leading and tailing spaces we want to know | ||
| - * whether the string is actually space padded. Required to communicate | ||
| - * with dumb peers which send space padded UIDs and fail if they | ||
| - * receive correct UIDs back. | ||
| - * | ||
| - * This test can only detect space padded strings if | ||
| - * dcmEnableAutomaticInputDataCorrection is false; otherwise the padding | ||
| - * has already been removed by dcmdata at this stage. | ||
| - */ | ||
| - size_t s_len = strlen(s); | ||
| - if ((s_len > 0)&&(s[s_len-1] == ' ')) *spacePadded = OFTrue; else *spacePadded = OFFalse; | ||
| + strncpy(s, aString, maxlen); | ||
| + if (spacePadded) | ||
| + { | ||
| + /* before we remove leading and tailing spaces we want to know | ||
| + * whether the string is actually space padded. Required to communicate | ||
| + * with dumb peers which send space padded UIDs and fail if they | ||
| + * receive correct UIDs back. | ||
| + * | ||
| + * This test can only detect space padded strings if | ||
| + * dcmEnableAutomaticInputDataCorrection is false; otherwise the padding | ||
| + * has already been removed by dcmdata at this stage. | ||
| + */ | ||
| + size_t s_len = strlen(s); | ||
| + if ((s_len > 0)&&(s[s_len-1] == ' ')) *spacePadded = OFTrue; else *spacePadded = OFFalse; | ||
| + } | ||
| + DU_stripLeadingAndTrailingSpaces(s); | ||
| } | ||
| - DU_stripLeadingAndTrailingSpaces(s); | ||
| } | ||
| } | ||
| return (ec.good())? ec : DIMSE_PARSEFAILED; |
60 changes: 60 additions & 0 deletions
60
recipes/dcmtk/all/patches/3.6.8-0003-fix-two-segmentation-faults.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| --- a/dcmdata/libsrc/dcelem.cc | ||
| +++ b/dcmdata/libsrc/dcelem.cc | ||
| @@ -717,6 +717,13 @@ OFCondition DcmElement::loadValue(DcmInputStream *inStream) | ||
| if (isStreamNew) | ||
| delete readStream; | ||
| } | ||
| + else | ||
| + { | ||
| + errorFlag = EC_InvalidStream; // incomplete dataset read from stream | ||
| + DCMDATA_ERROR("DcmElement: " << getTagName() << " " << getTag() | ||
| + << " larger (" << getLengthField() << ") than remaining bytes (" | ||
| + << getTransferredBytes() << ") in file, premature end of stream"); | ||
| + } | ||
| } | ||
| /* return result value */ | ||
| return errorFlag; | ||
| diff --git a/dcmnet/libsrc/dimcmd.cc b/dcmnet/libsrc/dimcmd.cc | ||
| index 6dca39546..ffd225f4b 100644 | ||
| --- a/dcmnet/libsrc/dimcmd.cc | ||
| +++ b/dcmnet/libsrc/dimcmd.cc | ||
| @@ -205,22 +205,25 @@ getString(DcmDataset *obj, DcmTagKey t, char *s, int maxlen, OFBool *spacePadded | ||
| return parseErrorWithMsg("dimcmd:getString: string too small", t); | ||
| } else { | ||
| ec = elem->getString(aString); | ||
| - strncpy(s, aString, maxlen); | ||
| - if (spacePadded) | ||
| + if (ec.good()) | ||
| { | ||
| - /* before we remove leading and tailing spaces we want to know | ||
| - * whether the string is actually space padded. Required to communicate | ||
| - * with dumb peers which send space padded UIDs and fail if they | ||
| - * receive correct UIDs back. | ||
| - * | ||
| - * This test can only detect space padded strings if | ||
| - * dcmEnableAutomaticInputDataCorrection is false; otherwise the padding | ||
| - * has already been removed by dcmdata at this stage. | ||
| - */ | ||
| - size_t s_len = strlen(s); | ||
| - if ((s_len > 0)&&(s[s_len-1] == ' ')) *spacePadded = OFTrue; else *spacePadded = OFFalse; | ||
| + strncpy(s, aString, maxlen); | ||
| + if (spacePadded) | ||
| + { | ||
| + /* before we remove leading and tailing spaces we want to know | ||
| + * whether the string is actually space padded. Required to communicate | ||
| + * with dumb peers which send space padded UIDs and fail if they | ||
| + * receive correct UIDs back. | ||
| + * | ||
| + * This test can only detect space padded strings if | ||
| + * dcmEnableAutomaticInputDataCorrection is false; otherwise the padding | ||
| + * has already been removed by dcmdata at this stage. | ||
| + */ | ||
| + size_t s_len = strlen(s); | ||
| + if ((s_len > 0)&&(s[s_len-1] == ' ')) *spacePadded = OFTrue; else *spacePadded = OFFalse; | ||
| + } | ||
| + DU_stripLeadingAndTrailingSpaces(s); | ||
| } | ||
| - DU_stripLeadingAndTrailingSpaces(s); | ||
| } | ||
| } | ||
| return (ec.good())? ec : DIMSE_PARSEFAILED; |
35 changes: 35 additions & 0 deletions
35
recipes/dcmtk/all/patches/3.6.8-0004-disable-oficonv-passthrough.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| --- a/config/docs/macros.txt | ||
| +++ b/config/docs/macros.txt | ||
| @@ -70,6 +70,20 @@ DCMTK_ENABLE_ACR_NEMA_DATASET_PRESENT_COMPATIBILITY | ||
| if backwards compatibility with ACR-NEMA is desired. If this is | ||
| important, compile with this macro enabled. | ||
|
|
||
| +DCMTK_ENABLE_ICONV_PASSTHROUGH | ||
| + Affected: oficonv | ||
| + Type of modification: Activates feature | ||
| + Explanation: If compiled with this macro, the oficonv libary will | ||
| + simply copy input to output during a character set conversion if | ||
| + input and output encoding are the same. This is more efficient, but | ||
| + means that in invalid byte sequences in the source will remain | ||
| + undetected and copied to the target, whereas the default behaviour | ||
| + would detect this and stop the conversion. It is also incompatible | ||
| + with the behaviour of the GNU iconv library. | ||
| + In DCMTK 3.6.8, this feature was always enabled. | ||
| + PATCH(conan-center-index): disabled by default in 3.6.8 as well to | ||
| + avoid regression https://support.dcmtk.org/redmine/issues/1143 | ||
| + | ||
| DCMTK_ENABLE_STRICT_HUFFMAN_TABLE_CHECK | ||
| Affected: dcmjpeg | ||
| Type of modification: Activates feature | ||
| --- a/oficonv/libsrc/citrus_iconv.c | ||
| +++ b/oficonv/libsrc/citrus_iconv.c | ||
| @@ -139,8 +139,7 @@ open_shared(struct _citrus_iconv_shared * * rci, | ||
| size_t len_convname; | ||
| int ret; | ||
|
|
||
| -#define INCOMPATIBLE_WITH_GNU_ICONV | ||
| -#ifdef INCOMPATIBLE_WITH_GNU_ICONV | ||
| +#ifdef DCMTK_ENABLE_ICONV_PASSTHROUGH | ||
| /* | ||
| * Use a pass-through when the (src,dest) encodings are the same. | ||
| */ |