Skip to content

Commit 969c4b6

Browse files
committed
Fixed race condition in file path resolution.
Fixed a time-of-check-to-time-of-use race condition in DcmJSONReader::loadBulkdataFile() that could be used by a malicious attacker to replace the input directory with a symbolic link pointing somewhere else, thus causing a file outside the permitted path to be read. Thanks to the IN-CYPHER OSS Security Team for the report, detailed analysis, proof of concept and proposed fix. This closes DCMTK Bug #1198.
1 parent ae94a3d commit 969c4b6

File tree

1 file changed

+21
-0
lines changed

1 file changed

+21
-0
lines changed

dcmdata/libsrc/dcjsonrd.cc

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,11 @@
3636
#include <windows.h>
3737
#endif
3838

39+
BEGIN_EXTERN_C
40+
#include <sys/stat.h>
41+
#include <fcntl.h>
42+
END_EXTERN_C
43+
3944
// Private creator identification string for files
4045
// containing a list of datasets as a private sequence
4146
#define JSON2DCM_PRIVATE_RESERVATION "JSON2DCM_LIST_OF_DATASETS"
@@ -1590,7 +1595,23 @@ OFCondition DcmJSONReader::loadBulkdataFile(
15901595
{
15911596
// open file for reading
15921597
OFFile file;
1598+
#ifdef _WIN32
15931599
if (! file.fopen(filepath, "rb"))
1600+
#else
1601+
// On Posix systems, use O_NOFOLLOW to prevent a race condition
1602+
// between the call to realpath() and this call that might allow
1603+
// a malicious attacker to replace the input directory with
1604+
// a symbolic link pointing somewhere else, thus causing a file
1605+
// outside the permitted path to be read. Since realpath()
1606+
// resolves all symbolic links, an error caused by O_NOFOLLOW
1607+
// can only occur in this situation.
1608+
int fd = open(filepath.c_str(), O_RDONLY | O_NOFOLLOW);
1609+
if (fd < 0)
1610+
{
1611+
return EC_InvalidFilename;
1612+
}
1613+
if (! file.fdopen(fd, "rb"))
1614+
#endif
15941615
{
15951616
OFString s("(unknown error code)");
15961617
file.getLastErrorString(s);

0 commit comments

Comments
 (0)