Skip to content

Commit

Permalink
Fix qpress directory traversal vulnerability (PierreLvx#6)
Browse files Browse the repository at this point in the history
A bad actor user can prepare the payload as:

```
mkdir -p AAAAAAAAA/secure_file_priv_dir
touch AAAAAAAAA/secure_file_priv_dir/evil.so
qpress -r AAAAAAAAA payload.qp
Then edit the payload.qp in a hex editor or sed to replace AAAAAAAAA with ../../../
(example: sed -i 's/AAAAAAAAA/..\/..\/..\//' payload.qp)
```

Fix bug by checking the directory and reject the command if find the attempt to traversal

Test: see example above and try to reproduce it. Before fix you can observe
traversal. After fix - the error message(File path contains directory traversal
which is not allowed.) shown, no traversal observe.

All new code of the whole pull request, including one or several files
that are either new files or modified ones, are contributed under the BSD-new
license.  I am contributing on behalf of my employer Amazon Web Services,
Inc.

Co-authored-by: Mikhail Chalov <mcchalov@amazon.com>
  • Loading branch information
Chaloff and Chaloff committed Aug 19, 2022
1 parent d45f9c5 commit ddb3120
Showing 1 changed file with 43 additions and 2 deletions.
45 changes: 43 additions & 2 deletions qpress.cpp
Expand Up @@ -183,7 +183,9 @@ unsigned long long bench_size;
unsigned long long recovery_file_written;
unsigned long long recovery_bad_bytes = 0;
unsigned long long current_file_payload;
char tmp[200000];
const long NAME_BUFFER_SIZE = 200000;
char tmp[NAME_BUFFER_SIZE];


enum {FATAL_ERROR, COUNTER_UPDATE, FILES_PROCESSED, RESULT, WARNING};

Expand Down Expand Up @@ -935,6 +937,21 @@ void compress_directory(string base_dir, string pattern)
}
}

bool check_if_path_has_dir_traversal(char * new_path)
{
bool ret_val = false;
char *found_template1 = NULL;
found_template1 = strstr (new_path,"../");
#ifdef WINDOWS
char *found_template2 = NULL;
found_template2 = strstr (new_path,"..\\");
ret_val = (found_template1 != NULL) || (found_template2 != NULL);
#else
ret_val = found_template1 != NULL;
#endif

return ret_val;
}

void decompress_directory(string extract_dir, bool std_out)
{
Expand All @@ -955,9 +972,21 @@ void decompress_directory(string extract_dir, bool std_out)
{
// read directory name, append it to current path and create the directory
chunk_size = fread32();
if(NAME_BUFFER_SIZE < chunk_size + 1)
{
abort("File path string is bigger than buffer size");
}
try_aread(tmp, chunk_size + 1);
curdir = curdir + DELIM_STR + tmp;
PRINT(FILES_PROCESSED, "%s%s%s\n", BLANK_LINE, remove_leading_curdir(curdir).c_str(), DELIM_STR);
/* RDS security fix: https://rds-jira.amazon.com/browse/MMCP-231
Need to check if we try to decompress file or directory outside working directory
*/
if(true == check_if_path_has_dir_traversal(tmp))
{
abort("Directory path contains directory traversal which is not allowed.");
}
/* End of RDS security fix*/
if(!std_out)
{
#ifdef WINDOWS
Expand All @@ -972,7 +1001,19 @@ void decompress_directory(string extract_dir, bool std_out)
else if(c == 'F')
{
chunk_size = fread32(); // read length of file name
try_aread(tmp, chunk_size + 1); // read file name
if(NAME_BUFFER_SIZE < chunk_size + 1)
{
abort("File path string is bigger than buffer size");
}
try_aread(tmp, chunk_size + 1); // read file name to 'tmp' with size 'chunk_size + 1'
/* RDS security fix: https://rds-jira.amazon.com/browse/MMCP-231
Need to check if we try to decompress file or directory outside working directory
*/
if(true == check_if_path_has_dir_traversal(tmp))
{
abort("File path contains directory traversal which is not allowed.");
}
/* End of RDS security fix*/
string buf2 = curdir + DELIM_STR + tmp;
PRINT(FILES_PROCESSED, "%s %s\n", BLANK_LINE, tmp);
if(!std_out)
Expand Down

0 comments on commit ddb3120

Please sign in to comment.