--- /dev/null
+Applied-Upstream: edbb085e45788dccaf0e64d71534cfca925784b8
+Author: Marco Eichelberg <eichelberg@offis.de>
+Last-Update: 2026-03-21
+Description: Sanitize all strings passed to the exec options.
+ Sanitize the text fields from incoming DICOM associations and DICOM objects
+ (such as Study Instance UID, SOP Instance UID, Patient's Name) and the
+ calling SCU's network presentation address by removing special characters
+ that may be interpreted as shell escape characters when one of the
+ execution options (e.g. --exec-on-reception) is in use.
+ .
+ Thanks to Machine Spirits UG (haftungsbeschränkt) for the bug report,
+ detailed analysis and proof of concept.
+ .
+ This closes DCMTK issue #1194.
+Bug: https://support.dcmtk.org/redmine/issues/1194
+Bug-Debian: https://bugs.debian.org/1133001
+Reviewed-By: Étienne Mollier <emollier@debian.org>
+
+--- dcmtk.orig/dcmnet/apps/storescp.cc
++++ dcmtk/dcmnet/apps/storescp.cc
+@@ -1,6 +1,6 @@
+ /*
+ *
+- * Copyright (C) 1994-2025, OFFIS e.V.
++ * Copyright (C) 1994-2026, OFFIS e.V.
+ * All rights reserved. See COPYRIGHT file for details.
+ *
+ * This software and supporting documentation were developed by
+@@ -1601,7 +1601,9 @@
+ calledAETitle.clear();
+ }
+ // store calling presentation address (i.e. remote hostname)
+- callingPresentationAddress = OFSTRING_GUARD(assoc->params->DULparams.callingPresentationAddress);
++ callingPresentationAddress = "\"";
++ callingPresentationAddress += OFSTRING_GUARD(assoc->params->DULparams.callingPresentationAddress);
++ callingPresentationAddress += "\"";
+
+ /* now do the real work, i.e. receive DIMSE commands over the network connection */
+ /* which was established and handle these commands correspondingly. In case of */
+@@ -1965,6 +1967,7 @@
+ dateTime.getTime().getHour(), dateTime.getTime().getMinute(), dateTime.getTime().getIntSecond(), dateTime.getTime().getMilliSecond());
+
+ OFString subdirectoryName;
++ OFString s;
+ switch (opt_sortStudyMode)
+ {
+ case ESM_Timestamp:
+@@ -1979,15 +1982,27 @@
+ subdirectoryName = opt_sortStudyDirPrefix;
+ if (!subdirectoryName.empty())
+ subdirectoryName += '_';
+- subdirectoryName += currentStudyInstanceUID;
+- OFStandard::sanitizeFilename(subdirectoryName);
++ s = currentStudyInstanceUID;
++ OFStandard::sanitizeFilename(s);
++ if (s != currentStudyInstanceUID)
++ {
++ OFLOG_WARN(storescpLogger, "Sanitized unusual characters in Study Instance UID, converted from \"" << currentStudyInstanceUID << "\" to \"" << s << "\".");
++ }
++ subdirectoryName += s;
+ break;
+ case ESM_PatientName:
+ // pattern: "[Patient's Name]_[YYYYMMDD]_[HHMMSSMMM]"
+ subdirectoryName = currentPatientName;
++ OFStandard::sanitizeFilename(subdirectoryName);
++ if (subdirectoryName != currentPatientName)
++ {
++ // It is quite normal that we need to sanitize characters in PatientName.
++ // Therefore, this is only a debug message and not a warning, unlike the other
++ // messages about sanitized fields, which are normally not expected.
++ OFLOG_DEBUG(storescpLogger, "Sanitized characters in Patient Name, converted from \"" << currentPatientName << "\" to \"" << subdirectoryName << "\".");
++ }
+ subdirectoryName += '_';
+ subdirectoryName += timestamp;
+- OFStandard::sanitizeFilename(subdirectoryName);
+ break;
+ case ESM_None:
+ break;
+@@ -2196,8 +2211,13 @@
+ else
+ {
+ // Use the SOP instance UID as found in the C-STORE request message as part of the filename
+- OFString uid(OFSTRING_GUARD(req->AffectedSOPInstanceUID));
++ OFString s(OFSTRING_GUARD(req->AffectedSOPInstanceUID));
++ OFString uid = s;
+ OFStandard::sanitizeFilename(uid);
++ if (uid != s)
++ {
++ OFLOG_WARN(storescpLogger, "Sanitized unusual characters in SOP Instance UID, converted from \"" << s << "\" to \"" << uid << "\".");
++ }
+ OFStandard::snprintf(imageFileName, sizeof(imageFileName), "%s%c%s.%s%s", opt_outputDirectory.c_str(), PATH_SEPARATOR, dcmSOPClassUIDToModality(req->AffectedSOPClassUID, "UNKNOWN"),
+ uid.c_str(), opt_fileNameExtension.c_str());
+ }
+@@ -2367,16 +2387,19 @@
+ if( !opt_ignore )
+ {
+ // perform substitution for placeholder #p (depending on presence of any --sort-xxx option)
++ // Note: We do not enclose this in quotes because it may be used as part of a path expression.
+ OFString dir = (opt_sortStudyMode == ESM_None) ? opt_outputDirectory : subdirectoryPathAndName;
+ cmd = replaceChars( cmd, OFString(PATH_PLACEHOLDER), dir );
+
+ // perform substitution for placeholder #f; note that outputFileNameArray.back()
+ // always contains the name of the file (without path) which was written last.
++ // Note: We do not enclose this in quotes because it may be used as part of a path expression.
+ OFString outputFileName = outputFileNameArray.back();
+ cmd = replaceChars( cmd, OFString(FILENAME_PLACEHOLDER), outputFileName );
+ }
+
+- // perform substitution for placeholder #a
++ // perform substitution for placeholder #a.
++ // Note that this string is already enclosed in double quotes at this point
+ s = callingAETitle;
+ sanitizeAETitle(s);
+ if (s != callingAETitle)
+@@ -2385,7 +2408,8 @@
+ }
+ cmd = replaceChars( cmd, OFString(CALLING_AETITLE_PLACEHOLDER), s );
+
+- // perform substitution for placeholder #c
++ // perform substitution for placeholder #c.
++ // Note that this string is already enclosed in double quotes at this point
+ s = calledAETitle;
+ sanitizeAETitle(s);
+ if (s != calledAETitle)
+@@ -2394,8 +2418,15 @@
+ }
+ cmd = replaceChars( cmd, OFString(CALLED_AETITLE_PLACEHOLDER), s );
+
+- // perform substitution for placeholder #r
+- cmd = replaceChars( cmd, OFString(CALLING_PRESENTATION_ADDRESS_PLACEHOLDER), callingPresentationAddress );
++ // perform substitution for placeholder #r.
++ // Note that this string is already enclosed in double quotes at this point
++ s = callingPresentationAddress;
++ sanitizeAETitle(s);
++ if (s != callingPresentationAddress)
++ {
++ OFLOG_WARN(storescpLogger, "Sanitized unusual characters in calling presentation address, converted from " << callingPresentationAddress << " to " << s << ".");
++ }
++ cmd = replaceChars( cmd, OFString(CALLING_PRESENTATION_ADDRESS_PLACEHOLDER), s );
+
+ // Execute command in a new process
+ executeCommand( cmd );
+@@ -2500,20 +2531,38 @@
+ OFString s;
+
+ // perform substitution for placeholder #p; #p will be substituted by lastStudySubdirectoryPathAndName
++ // Note: We do not enclose this in quotes because it may be used as part of a path expression.
+ cmd = replaceChars( cmd, OFString(PATH_PLACEHOLDER), lastStudySubdirectoryPathAndName );
+
+- // perform substitution for placeholder #a
++ // perform substitution for placeholder #a.
++ // Note that this string is already enclosed in double quotes at this point
+ s = callingAETitle;
+ sanitizeAETitle(s);
++ if (s != callingAETitle)
++ {
++ OFLOG_WARN(storescpLogger, "Sanitized unusual characters in calling aetitle, converted from " << callingAETitle << " to " << s << ".");
++ }
+ cmd = replaceChars( cmd, OFString(CALLING_AETITLE_PLACEHOLDER), s );
+
+- // perform substitution for placeholder #c
++ // perform substitution for placeholder #c.
++ // Note that this string is already enclosed in double quotes at this point
+ s = calledAETitle;
+ sanitizeAETitle(s);
++ if (s != calledAETitle)
++ {
++ OFLOG_WARN(storescpLogger, "Sanitized unusual characters in called aetitle, converted from " << calledAETitle << " to " << s << ".");
++ }
+ cmd = replaceChars( cmd, OFString(CALLED_AETITLE_PLACEHOLDER), s );
+
+- // perform substitution for placeholder #r
+- cmd = replaceChars( cmd, OFString(CALLING_PRESENTATION_ADDRESS_PLACEHOLDER), callingPresentationAddress );
++ // perform substitution for placeholder #r.
++ // Note that this string is already enclosed in double quotes at this point
++ s = callingPresentationAddress;
++ sanitizeAETitle(s);
++ if (s != callingPresentationAddress)
++ {
++ OFLOG_WARN(storescpLogger, "Sanitized unusual characters in calling presentation address, converted from " << callingPresentationAddress << " to " << s << ".");
++ }
++ cmd = replaceChars( cmd, OFString(CALLING_PRESENTATION_ADDRESS_PLACEHOLDER), s );
+
+ // Execute command in a new process
+ executeCommand( cmd );
+--- dcmtk.orig/ofstd/libsrc/ofstd.cc
++++ dcmtk/ofstd/libsrc/ofstd.cc
+@@ -3402,16 +3402,26 @@
+ }
+
+
++static const char sanitized_filename_charset[] =
++{
++ ' ', '_', '_', '_', '_', '_', '_', '_', '_', '_', '_', '_', '_', '-', '.', '_',
++ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', ':', '_', '_', '_', '_', '_',
++ '@', 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O',
++ 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', '_', '_', '_', '_', '_',
++ '_', 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o',
++ 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', '_', '_', '_', '_', '_'
++};
++
++
+ void OFStandard::sanitizeFilename(OFString& fname)
+ {
+ const size_t len = fname.length();
++ char c;
+ for (size_t i = 0; i < len; ++i)
+ {
+-#ifdef _WIN32
+- if ((fname[i] == PATH_SEPARATOR) || (fname[i] == '/')) fname[i] = '_';
+-#else
+- if (fname[i] == PATH_SEPARATOR) fname[i] = '_';
+-#endif
++ c = fname[i];
++ if (c != 0 && (c < 32 || c >= 127)) c = '_'; else c = sanitized_filename_charset[c-32];
++ fname[i] = c;
+ }
+ }
+
+@@ -3423,11 +3433,7 @@
+ char *c = fname;
+ while (*c)
+ {
+-#ifdef _WIN32
+- if ((*c == PATH_SEPARATOR) || (*c == '/')) *c = '_';
+-#else
+- if (*c == PATH_SEPARATOR) *c = '_';
+-#endif
++ if (*c < 32 || *c >= 127) *c = '_'; else *c = sanitized_filename_charset[*c-32];
+ ++c;
+ }
+ }
projects / dcmtk.git / commitdiff