Make clearer comments on why the current path checking suffices

Also fix that it `str_check_pathname` counted dots even if they were after a
non-dot character.
This commit is contained in:
heinrich5991 2015-02-26 12:14:28 +01:00 committed by oy
parent 6a1a0d800f
commit d335694931
3 changed files with 35 additions and 9 deletions

View file

@ -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;
}

View file

@ -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.

View file

@ -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;