diff --git a/src/base/system.c b/src/base/system.c index 2241e09e9..b818fb961 100644 --- a/src/base/system.c +++ b/src/base/system.c @@ -1613,20 +1613,34 @@ void str_sanitize_cc(char *str_in) /* check if the string contains '.' or '..' paths */ int str_check_pathname(const char* str) { - int parse_counter = 1; + // State machine. Non-negative numbers mean that we're at the beginning + // of a new directory/filename, and the number represents the number of + // dots ('.') we found. -1 means we encountered a different character + // since the last path separator (or the beginning of the string). + int parse_counter = 0; while(*str) { if(*str == '\\' || *str == '/') { - if(parse_counter >= 2 && parse_counter <= 3) + // A path separator. Check how many dots we found since + // the last path separator. + // + // One or two dots => "." or ".." contained in the + // path. Return an error. + if(parse_counter >= 1 && parse_counter <= 2) return -1; else - parse_counter = 1; + parse_counter = 0; + } + else if(parse_counter >= 0) + { + // If we have not encountered a non-dot character since + // the last path separator, count the dots. + if(*str == '.') + parse_counter++; + else + parse_counter = -1; } - else if(*str != '.') - parse_counter = 0; - else - ++parse_counter; ++str; } diff --git a/src/base/system.h b/src/base/system.h index 975dbd2b1..0fc015f83 100644 --- a/src/base/system.h +++ b/src/base/system.h @@ -815,9 +815,12 @@ void str_sanitize_cc(char *str); void str_sanitize(char *str); /* - Function: str_check_pathnam + Function: str_check_pathname Check if the string contains '.' or '..' paths. + NOTE: This does not check whether the path is absolute, other + checking must be in place for this case. + Parameters: str - String to check. diff --git a/src/engine/shared/storage.cpp b/src/engine/shared/storage.cpp index 5918faf8f..82511ced7 100644 --- a/src/engine/shared/storage.cpp +++ b/src/engine/shared/storage.cpp @@ -254,6 +254,8 @@ public: return pBuffer; } + // Open a file. This checks that the path appears to be a subdirectory + // of one of the storage paths. virtual IOHANDLE OpenFile(const char *pFilename, int Flags, int Type, char *pBuffer = 0, int BufferSize = 0) { char aBuffer[MAX_PATH_LENGTH]; @@ -263,7 +265,14 @@ public: BufferSize = sizeof(aBuffer); } - // check for valid path + // Check whether the path contains sole '..' or '.'. We'd + // normally still have to check whether it is an absolute path + // (starts with a path separator (or a drive name on windows)), + // but since we concatenate this path with another path later + // on, this can't become absolute. + // + // E. g. "/etc/passwd" => "/path/to/storage//etc/passwd", which + // is safe. if(str_check_pathname(pFilename) != 0) { pBuffer[0] = 0;