commit
e3878daf870cd2db50eadfde38615f0afae8a584
Author: Michael Onken <onken@open-connections.de>
Date: Tue May 19 17:16:08 2026 +0200
Fix path traversal in wlmscpfs through Called AET.
The wlmscpfs SCP appended the Called Application Entity Title received
in the A-ASSOCIATE-RQ directly onto the configured worklist data file
path and used the existence of the resulting directory as an access
control decision. Because DICOM VR AE permits the characters "/", "\"
and ".", a peer could send a 16-byte AE title such as "../secret/VICTIM"
and have wlmscpfs serve worklist records from a sibling directory of
the configured root. With option --request-file-path enabled, the same
AE title (and the Patient ID) was substituted into the output filename
template without sanitization, producing an arbitrary-location write
primitive outside the configured request file directory.
This commit closes both holes:
- wlmscpfs now rejects any A-ASSOCIATE-RQ whose Called AE title is not
safe to use as a single filesystem path component, refusing the
association with WLM_BAD_AE_SERVICE. The validation is implemented in
the new static method WlmFileSystemInteractionManager::IsValidAETitle\
ForFilesystem, which rejects empty or over-long titles, any title
containing a dot, and delegates the remaining character check to
OFStandard::sanitizeAETitle (sanitize-and-compare).
- The placeholder substitution in storeRequestToFile now passes each
substituted value (#a, #c, #p) through OFStandard::sanitizeAETitle
before insertion, and additionally sanitizes the final filename, so
that any path separator surviving the substitution is defanged.
Supporting changes:
- Promote storescp's private sanitizeAETitle helper into the public
OFStandard::sanitizeAETitle, with documentation noting that the
function is also used by wlmscpfs to validate filesystem path
components (so widening the allow list has downstream effects).
- Replace storescp's local copy of the helper with the new public one.
- Add a forward declaration of DcmSequenceOfItems in wlfsim.h that
was previously missing (existing callers happened to include dctk.h
first).
- Document the new behaviour in the wlmscpfs man page.
Tests:
- New ofstd_OFStandard_sanitizeAETitle test covers the lifted helper
(path separators, NUL, control bytes, high-range bytes, shell
metacharacters, the quotation-mark preservation behaviour, and
empty/length-1 edge cases).
- New dcmwlm_aetitle_validation test covers the validator directly
with every path-traversal payload from the bug report ("../secret/\
VICTIM", "../CARDIOLOGY", "..", ".", "..\secret"), dotted variants
("MY.AE", "foo..bar", ".foo", "foo."), structural rejections (empty,
17 bytes, embedded NUL, tab, 0xFF), shell metacharacters, and a row
of legitimate AE titles that must still be accepted.
- dcmwlm previously had no OFTEST scaffolding; tests.cc has been added
along with the corresponding CMakeLists.txt and Makefile.in entries.
Thanks to Abhinav Agarwal for the report.
Gbp-Pq: Name CVE-2026-52868.patch
static OFString replaceChars( const OFString &srcstr, const OFString &pattern, const OFString &substitute );
static void executeCommand( const OFString &cmd );
static void cleanChildren(pid_t pid, OFBool synch);
-static void sanitizeAETitle(OFString& aet);
static OFCondition acceptUnknownContextsWithPreferredTransferSyntaxes(
T_ASC_Parameters * params,
const char* transferSyntaxes[],
lastStudySubdirectoryPathAndName.clear();
}
-/* replace all characters that might be interpreted by the shell with underscores
- */
-static void sanitizeAETitle(OFString& aet)
-{
- static const char sanitized_aetitle_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', '_', '_', '_', '_', '_'
- };
-
- // the aet string starts and ends with quotation marks. We ignore these.
- size_t len = aet.length();
- if (len < 3) return;
-
- char c;
- --len;
- for (size_t i=1; i < len; ++i)
- {
- c = aet[i];
- if (c != 0 && (c < 32 || c >= 127)) c = '_'; else c = sanitized_aetitle_charset[c-32];
- aet[i] = c;
- }
-}
-
static void executeOnReception()
/*
* This function deals with the execution of the command line which was passed
// perform substitution for placeholder #a.
// Note that this string is already enclosed in double quotes at this point
s = callingAETitle;
- sanitizeAETitle(s);
+ OFStandard::sanitizeAETitle(s);
if (s != callingAETitle)
{
OFLOG_WARN(storescpLogger, "Sanitized unusual characters in calling aetitle, converted from " << callingAETitle << " to " << s << ".");
// perform substitution for placeholder #c.
// Note that this string is already enclosed in double quotes at this point
s = calledAETitle;
- sanitizeAETitle(s);
+ OFStandard::sanitizeAETitle(s);
if (s != calledAETitle)
{
OFLOG_WARN(storescpLogger, "Sanitized unusual characters in called aetitle, converted from " << calledAETitle << " to " << s << ".");
// perform substitution for placeholder #r.
// Note that this string is already enclosed in double quotes at this point
s = callingPresentationAddress;
- sanitizeAETitle(s);
+ OFStandard::sanitizeAETitle(s);
if (s != callingPresentationAddress)
{
OFLOG_WARN(storescpLogger, "Sanitized unusual characters in calling presentation address, converted from " << callingPresentationAddress << " to " << s << ".");
// perform substitution for placeholder #a.
// Note that this string is already enclosed in double quotes at this point
s = callingAETitle;
- sanitizeAETitle(s);
+ OFStandard::sanitizeAETitle(s);
if (s != callingAETitle)
{
OFLOG_WARN(storescpLogger, "Sanitized unusual characters in calling aetitle, converted from " << callingAETitle << " to " << s << ".");
// perform substitution for placeholder #c.
// Note that this string is already enclosed in double quotes at this point
s = calledAETitle;
- sanitizeAETitle(s);
+ OFStandard::sanitizeAETitle(s);
if (s != calledAETitle)
{
OFLOG_WARN(storescpLogger, "Sanitized unusual characters in called aetitle, converted from " << calledAETitle << " to " << s << ".");
// perform substitution for placeholder #r.
// Note that this string is already enclosed in double quotes at this point
s = callingPresentationAddress;
- sanitizeAETitle(s);
+ OFStandard::sanitizeAETitle(s);
if (s != callingPresentationAddress)
{
OFLOG_WARN(storescpLogger, "Sanitized unusual characters in calling presentation address, converted from " << callingPresentationAddress << " to " << s << ".");
Table K.6-1 in part 4 annex K of the DICOM standard lists all corresponding
type 1 attributes (see column "Return Key Type").
+The called Application Entity Title from an incoming A-ASSOCIATE-RQ is used
+as a subdirectory name below the \e --data-file-path directory in order to
+look up the worklist files that this association may access. The DICOM VR
+AE (see DICOM PS3.5 Section 6.1.3) permits the characters "/" (forward slash)
+and "." (full stop), so a conformant peer can send an AE title such as
+"../OTHER" that would resolve to a sibling directory of the configured
+worklist root. To prevent this, \b wlmscpfs rejects any A-ASSOCIATE-RQ
+whose called AE title is not safe to use as a single filesystem path
+component. The set of accepted characters is letters, digits, space and
+the characters "-", ":", "@" and "_"; any other byte (path separators
+"/" and "\\", NUL, control bytes, bytes outside the printable ASCII range,
+shell metacharacters as well as the dot character) causes the association
+to be rejected. Dots are rejected outright (rather than only the
+standalone components "." and "..") because real-world AE titles never
+contain dots and rejecting the entire character avoids any need to reason
+about platform-specific path normalization corner cases.
+
\subsection wlmscpfs_request_files Writing Request Files
Providing option \e --request-file-path enables writing of the incoming C-FIND
cannot be written successfully or will look broken once written. Also, an empty
Patient ID is used as such, i.e. the \#p will be replaced with an empty string.
+For security reasons, the values substituted for the placeholders \#a, \#c and
+\#p are sanitized before they are inserted into the resulting file name: path
+separators ("/", "\\") and other characters that are not safe in a file name
+component are replaced with the underscore character ("_"). This prevents
+attacker-controlled values transmitted over the network (called/calling AE
+title, Patient ID) from escaping the configured request file directory.
+
\subsection wlmscpfs_dicom_conformance DICOM Conformance
The \b wlmscpfs application supports the following SOP Classes as an SCP:
class DcmTagKey;
class OFCondition;
class DcmItem;
+class DcmSequenceOfItems;
class OFdirectory_iterator;
/** This class encapsulates data structures and operations for managing
/** Checks if the given called application entity title is supported. If this is the case,
* OFTrue will be returned, else OFFalse.
+ *
+ * The check is performed in two stages. First, the AE title is validated against the
+ * rules of IsValidAETitleForFilesystem(): titles that contain path separators, embedded
+ * NUL or other control characters, or any dot character are rejected outright (returning
+ * OFFalse). This prevents the wire-side AE title from escaping the worklist data file
+ * path through path traversal. Second, the AE title is appended to the configured
+ * worklist root path and the resulting directory must exist.
+ *
* @param calledApplicationEntityTitlev The application entity title which shall be checked
- * for support. Valid pointer expected.
+ * for support.
* @return OFTrue, if the called application entity title is supported,
- * OFFalse, if the called application entity title is not supported or it is not given.
+ * OFFalse, if the called application entity title is not supported, not given,
+ * or rejected because it would not be safe to use as a filesystem path component.
*/
OFBool IsCalledApplicationEntityTitleSupported( const OFString& calledApplicationEntityTitlev );
+ /** Determines whether the given DICOM Application Entity Title is safe to use as a single
+ * directory or filename component below the worklist data file path.
+ *
+ * An AE title is considered safe if and only if all of the following hold:
+ * - it is not empty and not longer than 16 bytes (the DICOM AE VR length limit),
+ * - it contains no dot ('.') at all,
+ * - OFStandard::sanitizeAETitle() would not change any byte of it.
+ *
+ * The third rule delegates the character-set check to OFStandard::sanitizeAETitle(),
+ * which is the same allow list used by \b storescp for filename and shell-substitution
+ * contexts: letters, digits, space and the characters '-', '.', ':', '@', '_' are kept,
+ * every other byte (path separators '/' and '\\', NUL, control characters, bytes
+ * outside the printable ASCII range and shell metacharacters) is replaced. If the
+ * sanitized string differs from the input, the AE title contains at least one such
+ * unsafe byte and is rejected.
+ *
+ * Dots are rejected by the explicit second rule even though sanitizeAETitle() keeps
+ * them. Real-world AE titles never contain dots, and rejecting the entire character
+ * avoids any need to reason about path normalization corner cases (".", "..", trailing
+ * dots on Windows etc.).
+ *
+ * Note that this delegation couples the rule applied here to the allow list inside
+ * OFStandard::sanitizeAETitle(). If that allow list is widened in the future, the set
+ * of AE titles accepted as safe filesystem path components widens too, so any change
+ * to it must consider downstream effects on \b wlmscpfs.
+ *
+ * @param aeTitle Application Entity Title to validate.
+ * @return OFTrue if the AE title is safe to use as a path component, OFFalse otherwise.
+ */
+ static OFBool IsValidAETitleForFilesystem( const OFString& aeTitle );
+
/** This function determines the records from the Worklist files that match
* the given search mask and returns the number of matching records. Also,
* this function will store the matching records inside the member variable
/*
*
- * Copyright (C) 1996-2024, OFFIS e.V.
+ * Copyright (C) 1996-2026, OFFIS e.V.
* All rights reserved. See COPYRIGHT file for details.
*
* This software and supporting documentation were developed by
// ----------------------------------------------------------------------------
+OFBool WlmFileSystemInteractionManager::IsValidAETitleForFilesystem( const OFString& aeTitle )
+{
+ // Length must be 1..16 (DICOM AE VR limit).
+ if( aeTitle.empty() || aeTitle.length() > 16 )
+ return OFFalse;
+
+ // OFStandard::sanitizeAETitle() keeps '.' in its allow list (the function
+ // is also used in shell-substitution contexts where dots are common in
+ // AE titles), so reject dots explicitly here. Rejecting the entire
+ // character avoids any need to reason about platform-specific path
+ // normalization corner cases (".", "..", trailing dots on Windows etc.).
+ if( aeTitle.find('.') != OFString_npos )
+ return OFFalse;
+
+ // Delegate the remaining character check to OFStandard::sanitizeAETitle().
+ // If the sanitizer would change any byte, the AE title contains characters
+ // that are not safe to use as a single filesystem path component (path
+ // separators, NUL, control bytes, bytes outside the printable ASCII range
+ // and shell metacharacters).
+ OFString sanitized = aeTitle;
+ OFStandard::sanitizeAETitle( sanitized );
+ return ( sanitized == aeTitle ) ? OFTrue : OFFalse;
+}
+
+// ----------------------------------------------------------------------------
+
OFBool WlmFileSystemInteractionManager::IsCalledApplicationEntityTitleSupported( const OFString& calledApplicationEntityTitlev )
// Date : July 11, 2002
// Author : Thomas Wilkens
// copy value
calledApplicationEntityTitle = calledApplicationEntityTitlev;
+ // Reject AE titles that are not safe to use as a filesystem path component.
+ // This guards against path traversal via path separators or "../" segments
+ // smuggled in through the wire-side AE title.
+ if( !IsValidAETitleForFilesystem( calledApplicationEntityTitle ) )
+ {
+ DCMWLM_WARN( "Refusing called AE title because it is not safe to use as a directory name (contains path separators, control characters or is \".\"/\"..\"); rejecting association" );
+ return( OFFalse );
+ }
+
// Determine complete path to the files that make up the data source.
OFString fullPath( dfPath );
if( !fullPath.empty() && fullPath[fullPath.length()-1] != PATH_SEPARATOR )
static void storeRequestToFile(DcmDataset& request, const OFString& callingAE, const OFString& calledAE, const OFString& reqFilePath, const OFString& reqFileFormat)
{
OFString fileName = reqFileFormat;
- // Called Application Entity Title
- OFStringUtil::replace_all(fileName, WLM_CALLED_AETITLE_PLACEHOLDER, calledAE);
- // Calling Application Entity Title
- OFStringUtil::replace_all(fileName, WLM_CALLING_AETITLE_PLACEHOLDER, callingAE);
+
+ // Each substituted value is sanitized before being inserted into the
+ // filename template. This prevents the calling/called AE title and the
+ // Patient ID (all originating on the wire) from injecting path
+ // separators or other unsafe characters into the resulting filename
+ // and thereby escaping the configured request file directory.
+ OFString safeCalledAE = calledAE;
+ OFStandard::sanitizeAETitle(safeCalledAE);
+ OFStringUtil::replace_all(fileName, WLM_CALLED_AETITLE_PLACEHOLDER, safeCalledAE);
+
+ OFString safeCallingAE = callingAE;
+ OFStandard::sanitizeAETitle(safeCallingAE);
+ OFStringUtil::replace_all(fileName, WLM_CALLING_AETITLE_PLACEHOLDER, safeCallingAE);
// Process ID
int processID = dcmtk::log4cplus::internal::get_process_id();
// Patient ID goes last since it might contain placeholders again (".#x...)"
OFString patientID;
request.findAndGetOFStringArray(DCM_PatientID, patientID);
+ OFStandard::sanitizeAETitle(patientID);
OFStringUtil::replace_all(fileName, WLM_PATIENT_ID_PLACEHOLDER, patientID);
+ // Defense in depth: also sanitize the fully assembled filename so that
+ // any path separator that survived (for example through interaction
+ // between placeholder substitutions or via the user-supplied format
+ // string itself) is replaced before the file is opened.
+ OFStandard::sanitizeAETitle(fileName);
+
// Finally store file
STD_NAMESPACE ofstream outputStream;
OFString fullPath;
# declare executables
DCMTK_ADD_TEST_EXECUTABLE(wltest wltest.cc)
+DCMTK_ADD_TEST_EXECUTABLE(dcmwlm_tests tests.cc twlaetval.cc)
# make sure executables are linked to the corresponding libraries
DCMTK_TARGET_LINK_MODULES(wltest dcmwlm dcmtls)
+DCMTK_TARGET_LINK_MODULES(dcmwlm_tests dcmwlm)
+
+# This macro parses tests.cc and registers all tests
+DCMTK_ADD_TESTS(dcmwlm)
LOCALLIBS = -ldcmwlm -ldcmnet -ldcmdata -loflog -lofstd -loficonv $(ZLIBLIBS) \
$(TCPWRAPPERLIBS) $(CHARCONVLIBS) $(MATHLIBS)
-objs = wltest.o
-progs = wltest
+test_objs = tests.o twlaetval.o
+objs = wltest.o $(test_objs)
+progs = wltest dcmwlm_tests
all: $(progs)
-wltest: $(objs)
- $(CXX) $(CXXFLAGS) $(LIBDIRS) $(LDFLAGS) -o $@ $(objs) $(LOCALLIBS) $(LIBS)
+wltest: wltest.o
+ $(CXX) $(CXXFLAGS) $(LIBDIRS) $(LDFLAGS) -o $@ wltest.o $(LOCALLIBS) $(LIBS)
+dcmwlm_tests: $(test_objs)
+ $(CXX) $(CXXFLAGS) $(LIBDIRS) $(LDFLAGS) -o $@ $(test_objs) $(LOCALLIBS) $(LIBS)
-check:
-check-exhaustive:
+check: dcmwlm_tests
+ ./dcmwlm_tests
+
+check-exhaustive: dcmwlm_tests
+ ./dcmwlm_tests -x
install: all
--- /dev/null
+/*
+ *
+ * Copyright (C) 2026, OFFIS e.V.
+ * All rights reserved. See COPYRIGHT file for details.
+ *
+ * This software and supporting documentation were developed by
+ *
+ * OFFIS e.V.
+ * R&D Division Health
+ * Escherweg 2
+ * D-26121 Oldenburg, Germany
+ *
+ *
+ * Module: dcmwlm
+ *
+ * Author: Michael Onken
+ *
+ * Purpose: main test program
+ *
+ */
+
+#include "dcmtk/config/osconfig.h"
+#include "dcmtk/ofstd/oftest.h"
+
+OFTEST_REGISTER(dcmwlm_aetitle_validation);
+
+OFTEST_MAIN("dcmwlm")
--- /dev/null
+/*
+ *
+ * Copyright (C) 2026, OFFIS e.V.
+ * All rights reserved. See COPYRIGHT file for details.
+ *
+ * This software and supporting documentation were developed by
+ *
+ * OFFIS e.V.
+ * R&D Division Health
+ * Escherweg 2
+ * D-26121 Oldenburg, Germany
+ *
+ *
+ * Module: dcmwlm
+ *
+ * Author: Michael Onken
+ *
+ * Purpose: Tests for the AE title validation used by the wlmscpfs
+ * filesystem worklist SCP to guard against path traversal
+ * through the Called Application Entity Title.
+ *
+ */
+
+#include "dcmtk/config/osconfig.h"
+
+#include "dcmtk/ofstd/oftest.h"
+#include "dcmtk/ofstd/ofstring.h"
+#include "dcmtk/dcmwlm/wlfsim.h"
+
+
+// Helper: build an OFString that contains a single NUL byte in the middle
+// of an otherwise printable AE title. OFString tolerates embedded NUL bytes
+// when constructed from a (pointer, length) pair, which is how a NUL byte
+// could enter wlmscpfs from the wire.
+static OFString aeTitleWithEmbeddedNul()
+{
+ const char raw[] = { 'A', 'B', '\0', 'C', 'D' };
+ return OFString(raw, sizeof(raw));
+}
+
+
+OFTEST(dcmwlm_aetitle_validation)
+{
+ // ----- Path traversal payloads from the original bug report. -----
+ // All of these must be rejected; if any of them is accepted, the
+ // CVE-2026-... path traversal hole is open again.
+
+ // 16-byte payload from the proof of concept ("../secret/VICTIM").
+ OFCHECK(!WlmFileSystemInteractionManager::IsValidAETitleForFilesystem("../secret/VICTIM"));
+
+ // Multi-AET demo ("../CARDIOLOGY").
+ OFCHECK(!WlmFileSystemInteractionManager::IsValidAETitleForFilesystem("../CARDIOLOGY"));
+
+ // Plain parent reference and self reference.
+ OFCHECK(!WlmFileSystemInteractionManager::IsValidAETitleForFilesystem(".."));
+ OFCHECK(!WlmFileSystemInteractionManager::IsValidAETitleForFilesystem("."));
+
+ // Windows-style separator.
+ OFCHECK(!WlmFileSystemInteractionManager::IsValidAETitleForFilesystem("..\\secret"));
+
+ // Leading-dot variants and dots in the middle. None of these are
+ // path-traversal payloads on their own, but we reject any AE title
+ // that contains a dot, see the documentation of
+ // IsValidAETitleForFilesystem() for the rationale.
+ OFCHECK(!WlmFileSystemInteractionManager::IsValidAETitleForFilesystem(".foo"));
+ OFCHECK(!WlmFileSystemInteractionManager::IsValidAETitleForFilesystem("foo."));
+ OFCHECK(!WlmFileSystemInteractionManager::IsValidAETitleForFilesystem("foo..bar"));
+ OFCHECK(!WlmFileSystemInteractionManager::IsValidAETitleForFilesystem("MY.AE"));
+
+ // ----- Other unsafe input. -----
+
+ // Empty AE title.
+ OFCHECK(!WlmFileSystemInteractionManager::IsValidAETitleForFilesystem(""));
+
+ // Too long (17 bytes; the DICOM AE VR allows at most 16).
+ OFCHECK(!WlmFileSystemInteractionManager::IsValidAETitleForFilesystem("ABCDEFGHIJKLMNOPQ"));
+
+ // Embedded NUL byte.
+ OFCHECK(!WlmFileSystemInteractionManager::IsValidAETitleForFilesystem(aeTitleWithEmbeddedNul()));
+
+ // Control character (tab).
+ OFCHECK(!WlmFileSystemInteractionManager::IsValidAETitleForFilesystem("AB\tCD"));
+
+ // High-range byte.
+ {
+ OFString withHigh;
+ withHigh.append("AE", 2);
+ withHigh.append(1, OFstatic_cast(char, 0xFF));
+ OFCHECK(!WlmFileSystemInteractionManager::IsValidAETitleForFilesystem(withHigh));
+ }
+
+ // Shell metacharacters: rejected because
+ // OFStandard::sanitizeAETitle() would replace them.
+ OFCHECK(!WlmFileSystemInteractionManager::IsValidAETitleForFilesystem("AE;rm"));
+ OFCHECK(!WlmFileSystemInteractionManager::IsValidAETitleForFilesystem("AE|cat"));
+ OFCHECK(!WlmFileSystemInteractionManager::IsValidAETitleForFilesystem("AE`id`"));
+ OFCHECK(!WlmFileSystemInteractionManager::IsValidAETitleForFilesystem("AE$VAR"));
+
+ // ----- Legitimate AE titles must still be accepted. -----
+
+ // Plain alphanumeric, mixed case.
+ OFCHECK(WlmFileSystemInteractionManager::IsValidAETitleForFilesystem("WORKLIST"));
+ OFCHECK(WlmFileSystemInteractionManager::IsValidAETitleForFilesystem("CARDIOLOGY"));
+ OFCHECK(WlmFileSystemInteractionManager::IsValidAETitleForFilesystem("MyAE"));
+
+ // Single character (minimum length).
+ OFCHECK(WlmFileSystemInteractionManager::IsValidAETitleForFilesystem("A"));
+
+ // Maximum length (16 bytes).
+ OFCHECK(WlmFileSystemInteractionManager::IsValidAETitleForFilesystem("ABCDEFGHIJKLMNOP"));
+
+ // Conformant punctuation that is also safe as a path component.
+ OFCHECK(WlmFileSystemInteractionManager::IsValidAETitleForFilesystem("MY-AE_1"));
+ OFCHECK(WlmFileSystemInteractionManager::IsValidAETitleForFilesystem("AE@HOST"));
+ OFCHECK(WlmFileSystemInteractionManager::IsValidAETitleForFilesystem("AE:1"));
+}
*/
static void sanitizeFilename(char *fname);
+ /** sanitize a DICOM Application Entity Title for safe use as a
+ * substitution value in a filename or in a command line that will
+ * be passed to a shell.
+ *
+ * This method maps each byte through an allow list: ASCII letters,
+ * digits, space and the characters '-', '.', ':', '@', '_' are kept
+ * unchanged; every other byte (including path separators '/' and
+ * '\\', NUL, control characters, shell metacharacters and bytes
+ * outside the printable ASCII range) is replaced by '_'.
+ *
+ * The aetitle string is expected to be already trimmed (i.e. without
+ * leading or trailing whitespace). Surrounding quotation marks, if
+ * present, are preserved unchanged (the first and last byte are not
+ * rewritten) so that quoted AE titles intended for shell command
+ * substitution remain quoted.
+ *
+ * This sanitization is sufficient for preventing shell injection in
+ * contexts where the AE title is substituted into an already quoted
+ * command-line argument, and for preventing path separators from
+ * appearing in filenames. It is NOT sufficient to prevent path
+ * traversal on its own, because the character '.' is kept and the
+ * sequence "..", which is a path-relative parent reference on most
+ * operating systems, is therefore not removed. Callers that
+ * substitute the result into a filesystem path must reject or
+ * collapse such sequences explicitly.
+ *
+ * Note: this function is also used by the wlmscpfs application as
+ * the basis of its filesystem-path-component validation (an AE title
+ * is accepted only if sanitizeAETitle() would not change it, in
+ * addition to an explicit dot rejection). Widening the allow list
+ * below therefore widens the set of AE titles accepted as worklist
+ * directory names; any change to it must consider that downstream
+ * effect.
+ *
+ * @param aetitle Application Entity Title to be sanitized in place.
+ */
+ static void sanitizeAETitle(OFString& aetitle);
+
/** retrieve the name of the default directory for support data.
* On Windows, this method resolves environment variables such as
* \%PROGRAMDATA% in the path, on Posix platforms it just returns
}
+// Allow list used by sanitizeAETitle(). Index is (byte - 32), so the
+// table covers the printable ASCII range 0x20..0x7E. Every entry either
+// repeats the input byte (kept) or is '_' (replaced). Kept characters:
+// space, '-', '.', ':', '@', '_' and ASCII letters/digits. All shell
+// metacharacters and path separators map to '_'.
+static const char sanitized_aetitle_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::sanitizeAETitle(OFString& aetitle)
+{
+ // Preserve a surrounding pair of quotation marks (used by callers
+ // that substitute the AE title into an already quoted shell argument).
+ size_t len = aetitle.length();
+ size_t start = 0;
+ if (len >= 2 && aetitle[0] == '"' && aetitle[len - 1] == '"')
+ {
+ start = 1;
+ --len;
+ }
+ for (size_t i = start; i < len; ++i)
+ {
+ unsigned char c = OFstatic_cast(unsigned char, aetitle[i]);
+ if (c < 32 || c >= 127) aetitle[i] = '_';
+ else aetitle[i] = sanitized_aetitle_charset[c - 32];
+ }
+}
+
+
OFString OFStandard::getDefaultSupportDataDir()
{
#ifdef HAVE_WINDOWS_H
/*
*
- * Copyright (C) 2011-2025, OFFIS e.V.
+ * Copyright (C) 2011-2026, OFFIS e.V.
* All rights reserved. See COPYRIGHT file for details.
*
* This software and supporting documentation were developed by
OFTEST_REGISTER(ofstd_OFStandard_isReadWriteable);
OFTEST_REGISTER(ofstd_OFStandard_appendFilenameExtension);
OFTEST_REGISTER(ofstd_OFStandard_removeRootDirFromPathname);
+OFTEST_REGISTER(ofstd_OFStandard_sanitizeAETitle);
OFTEST_REGISTER(ofstd_OFFile);
OFTEST_REGISTER(ofstd_OFString_compare);
OFTEST_REGISTER(ofstd_OFString_concatenate);
/*
*
- * Copyright (C) 2002-2022, OFFIS e.V.
+ * Copyright (C) 2002-2026, OFFIS e.V.
* All rights reserved. See COPYRIGHT file for details.
*
* This software and supporting documentation were developed by
OFCHECK(OFStandard::removeRootDirFromPathname(result, nullPtr, nullPtr).good());
}
+OFTEST(ofstd_OFStandard_sanitizeAETitle)
+{
+ // Pure-ASCII alphanumeric is preserved.
+ OFString s = "MY_AE_42";
+ OFStandard::sanitizeAETitle(s);
+ OFCHECK_EQUAL(s, "MY_AE_42");
+
+ // Dots, colons, dashes, '@' are part of the allow list.
+ s = "AE.TITLE:1@host-1";
+ OFStandard::sanitizeAETitle(s);
+ OFCHECK_EQUAL(s, "AE.TITLE:1@host-1");
+
+ // Path separators map to underscores.
+ s = "../etc/passwd";
+ OFStandard::sanitizeAETitle(s);
+ OFCHECK_EQUAL(s, ".._etc_passwd");
+
+ // Backslash is replaced (Windows path separator).
+ s = "..\\secret";
+ OFStandard::sanitizeAETitle(s);
+ OFCHECK_EQUAL(s, ".._secret");
+
+ // Shell metacharacters are replaced.
+ s = "AE;rm -rf /";
+ OFStandard::sanitizeAETitle(s);
+ OFCHECK_EQUAL(s, "AE_rm -rf _");
+
+ // Control characters (including NUL inside the string) are replaced.
+ OFString withNul;
+ withNul.append("AB", 2);
+ withNul.append(1, '\0');
+ withNul.append("CD", 2);
+ OFStandard::sanitizeAETitle(withNul);
+ OFCHECK_EQUAL(withNul, OFString("AB_CD"));
+
+ // High-range bytes (>= 0x7F) are replaced.
+ OFString withHigh;
+ withHigh.append("AE", 2);
+ withHigh.append(1, OFstatic_cast(char, 0xFF));
+ OFStandard::sanitizeAETitle(withHigh);
+ OFCHECK_EQUAL(withHigh, OFString("AE_"));
+
+ // A surrounding quotation-mark pair is preserved (storescp behaviour).
+ s = "\"../bad\"";
+ OFStandard::sanitizeAETitle(s);
+ OFCHECK_EQUAL(s, "\".._bad\"");
+
+ // Empty string and length-1 string are handled without crashing.
+ s = "";
+ OFStandard::sanitizeAETitle(s);
+ OFCHECK_EQUAL(s, "");
+ s = "/";
+ OFStandard::sanitizeAETitle(s);
+ OFCHECK_EQUAL(s, "_");
+}
+
OFTEST(ofstd_safeSubtractAddMult)
{
// --------------- Subtraction ----------------
projects / dcmtk.git / commitdiff