open-dsi
/
clamav
mirror of https://github.com/Cisco-Talos/clamav
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

bb12284 - Fix to prevent path traversal when using cli_genfname() to generate filenames that may retain path and filename information. Changed scanrar so that it will no longer retain path information for extracted files.

Browse Source
pull/111/head
Micah Snyder 6 years ago
parent a8ca96687a
commit 155eaaad8b
10 changed files with 313 additions and 27 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 2
      libclamav/libclamav.map
  2. 37
      libclamav/others.h
  3. 126
      libclamav/others_common.c
  4. 2
      libclamav/scanners.c
  5. 42
      libclamav/str.c
  6. 12
      libclamav/str.h
  7. 1
      m4/reorganization/code_checks/functions.m4
  8. 106
      unit_tests/check_clamav.c
  9. 6
      win32/clamav-config.h
  10. 6
      win32/update-win32.pl

2
libclamav/libclamav.map
Unescape View File

@ -116,6 +116,7 @@ CLAMAV_PRIVATE {
cli_memstr;
cli_strdup;
cli_strndup;
cli_strnstr;
cli_realloc;
cli_ctime;
tableCreate;
@ -257,6 +258,7 @@ CLAMAV_PRIVATE {
cl_get_pkey_file;
cl_base64_decode;
cl_base64_encode;
cli_sanitize_filepath;
local:
*;
};

37
libclamav/others.h
Unescape View File

@ -754,20 +754,29 @@ int cli_readn(int fd, void *buff, unsigned int count);
int cli_writen(int fd, const void *buff, unsigned int count);
const char *cli_gettmpdir(void);
/**
* @brief Sanitize a relative path, so it cannot have a negative depth.
*
* Caller is responsible for freeing the filename.
*
* @return char* filename or NULL.
*/
char *cli_sanitize_filepath(const char *filepath, size_t filepath_len);
/**
* @brief Generate tempfile filename (no path) with a random MD5 hash.
*
*
* Caller is responsible for freeing the filename.
*
*
* @return char* filename or NULL.
*/
char *cli_genfname(const char *prefix);
/**
* @brief Generate a full tempfile filepath with a random MD5 hash and prefix the name, if provided.
*
*
* Caller is responsible for freeing the filename.
*
*
* @param dir Alternative temp directory. (optional)
* @return char* filename or NULL.
*/
@ -775,9 +784,9 @@ char *cli_gentemp_with_prefix(const char *dir, const char *prefix);
/**
* @brief Generate a full tempfile filepath with a random MD5 hash.
*
*
* Caller is responsible for freeing the filename.
*
*
* @param dir Alternative temp directory. (optional)
* @return char* filename or NULL.
*/
@ -789,18 +798,18 @@ char *cli_gentemp(const char *dir);
* @param dir Alternative temp directory (optional).
* @param[out] name Allocated filepath, must be freed by caller.
* @param[out] fd File descriptor of open temp file.
* @return cl_error_t CL_SUCCESS, CL_ECREAT, or CL_EMEM.
* @return cl_error_t CL_SUCCESS, CL_ECREAT, or CL_EMEM.
*/
cl_error_t cli_gentempfd(const char *dir, char **name, int *fd);
/**
* @brief Create a temp filename, create the file, open it, and pass back the filepath and open file descriptor.
*
*
* @param dir Alternative temp directory (optional).
* @param prefix (Optional) Prefix for new file tempfile.
* @param[out] name Allocated filepath, must be freed by caller.
* @param[out] fd File descriptor of open temp file.
* @return cl_error_t CL_SUCCESS, CL_ECREAT, or CL_EMEM.
* @return cl_error_t CL_SUCCESS, CL_ECREAT, or CL_EMEM.
*/
cl_error_t cli_gentempfd_with_prefix(const char *dir, char *prefix, char **name, int *fd);
@ -849,11 +858,11 @@ struct cli_ftw_cbdata {
void *data;
};
/*
/*
* return CL_BREAK to break out without an error, CL_SUCCESS to continue,
* or any CL_E* to break out due to error.
* The callback is responsible for freeing filename when it is done using it.
* Note that callback decides if directory traversal should continue
* Note that callback decides if directory traversal should continue
* after an error, we call the callback with reason == error,
* and if it returns CL_BREAK we break.
*/
@ -866,7 +875,7 @@ typedef int (*cli_ftw_cb)(STATBUF *stat_buf, char *filename, const char *path, e
typedef int (*cli_ftw_pathchk)(const char *path, struct cli_ftw_cbdata *data);
/*
* returns
* returns
* CL_SUCCESS if it traversed all files and subdirs
* CL_BREAK if traversal has stopped at some point
* CL_E* if error encountered during traversal and we had to break out
@ -885,10 +894,10 @@ const char *cli_strerror(int errnum, char *buf, size_t len);
/**
* @brief Attempt to get a filename from an open file descriptor.
*
*
* Caller is responsible for free'ing the filename.
* Should work on Linux, macOS, Windows.
*
*
* @param desc File descriptor
* @param[out] filepath Will be set to file path if found, or NULL.
* @return cl_error_t CL_SUCCESS if found, else an error code.

126
libclamav/others_common.c
Unescape View File

@ -55,6 +55,7 @@
#include "clamav.h"
#include "others.h"
#include "platform.h"
#include "regex/regex.h"
#include "ltdl.h"
#include "matcher-ac.h"
@ -852,16 +853,132 @@ unsigned int cli_rndnum(unsigned int max)
return 1 + (unsigned int)(max * (rand() / (1.0 + RAND_MAX)));
}
char *cli_sanitize_filepath(const char *filepath, size_t filepath_len)
{
uint32_t depth = 0;
size_t index = 0;
size_t sanitized_index = 0;
char *sanitized_filepath = NULL;
if ((NULL == filepath) || (0 == filepath_len) || (MAX_PATH < filepath_len)) {
goto done;
}
sanitized_filepath = cli_calloc(filepath_len + 1, sizeof(unsigned char));
if (NULL == sanitized_filepath) {
cli_dbgmsg("cli_sanitize_filepath: out of memory\n");
goto done;
}
while (index < filepath_len) {
char *next_pathsep = NULL;
if (0 == strncmp(filepath + index, PATHSEP, strlen(PATHSEP))) {
/*
* Is "/" (or "\\" on Windows)
*/
/* Skip leading pathsep in absolute path, or extra pathsep) */
index += strlen(PATHSEP);
continue;
} else if (0 == strncmp(filepath + index, "." PATHSEP, strlen("." PATHSEP))) {
/*
* Is "./" (or ".\\" on Windows)
*/
/* Current directory indicator is meaningless and should not add to the depth. Skip it. */
index += strlen("." PATHSEP);
continue;
} else if (0 == strncmp(filepath + index, ".." PATHSEP, strlen(".." PATHSEP))) {
/*
* Is "../" (or "..\\" on Windows)
*/
if (depth == 0) {
/* Relative path would traverse parent directory. Skip it. */
index += strlen(".." PATHSEP);
continue;
} else {
/* Relative path is safe. Allow it. */
strncpy(sanitized_filepath + sanitized_index, filepath + index, strlen(".." PATHSEP));
sanitized_index += strlen(".." PATHSEP);
index += strlen(".." PATHSEP);
depth--;
}
#ifdef _WIN32
/*
* Windows' POSIX style API's accept both "/" and "\\" style path separators.
* The following checks using POSIX style path separators on Windows.
*/
} else if (0 == strncmp(filepath + index, "/", strlen("/"))) {
/*
* Is "/".
*/
/* Skip leading pathsep in absolute path, or extra pathsep) */
index += strlen("/");
continue;
} else if (0 == strncmp(filepath + index, "./", strlen("./"))) {
/*
* Is "./"
*/
/* Current directory indicator is meaningless and should not add to the depth. Skip it. */
index += strlen("./");
continue;
} else if (0 == strncmp(filepath + index, "../", strlen("../"))) {
/*
* Is "../"
*/
if (depth == 0) {
/* Relative path would traverse parent directory. Skip it. */
index += strlen("../");
continue;
} else {
/* Relative path is safe. Allow it. */
strncpy(sanitized_filepath + sanitized_index, filepath + index, strlen("../"));
sanitized_index += strlen("../");
index += strlen("../");
depth--;
}
#endif
} else {
/*
* Is not "/", "./", or "../".
*/
/* Find the next path separator. */
next_pathsep = cli_strnstr(filepath + index, PATHSEP, filepath_len - index);
if (NULL == next_pathsep) {
/* No more path separators, copy the rest (filename) into the sanitized path */
strncpy(sanitized_filepath + sanitized_index, filepath + index, filepath_len - index);
break;
}
next_pathsep += strlen(PATHSEP); /* Include the path separator in the copy */
/* Copy next directory name into the sanitized path */
strncpy(sanitized_filepath + sanitized_index, filepath + index, next_pathsep - (filepath + index));
sanitized_index += next_pathsep - (filepath + index);
index += next_pathsep - (filepath + index);
depth++;
}
}
done:
if ((NULL != sanitized_filepath) && (0 == strlen(sanitized_filepath))) {
free(sanitized_filepath);
sanitized_filepath = NULL;
}
return sanitized_filepath;
}
char *cli_genfname(const char *prefix)
{
char *fname;
char *sanitized_prefix = NULL;
char *fname = NULL;
unsigned char salt[16 + 32];
char *tmp;
int i;
size_t len;
if (prefix && (strlen(prefix) > 0)) {
len = strlen(prefix) + 1 + 5 + 1; /* {prefix}.{5}\0 */
sanitized_prefix = cli_sanitize_filepath(prefix, strlen(prefix));
len = strlen(sanitized_prefix) + 1 + 5 + 1; /* {prefix}.{5}\0 */
} else {
len = 6 + 1 + 48 + 4 + 1; /* clamav-{48}.tmp\0 */
}
@ -893,9 +1010,10 @@ char *cli_genfname(const char *prefix)
return NULL;
}
if (prefix && (strlen(prefix) > 0)) {
if (sanitized_prefix && (strlen(sanitized_prefix) > 0)) {
fname[5] = '\0';
snprintf(fname, len, "%s.%s", prefix, tmp);
snprintf(fname, len, "%s.%s", sanitized_prefix, tmp);
free(sanitized_prefix);
} else {
snprintf(fname, len, "clamav-%s.tmp", tmp);
}

2
libclamav/scanners.c
Unescape View File

@ -431,7 +431,7 @@ static cl_error_t cli_scanrar(const char *filepath, int desc, cli_ctx *ctx)
/*
* Extract the file...
*/
extract_fullpath = cli_gentemp_with_prefix(extract_dir, metadata.filename);
extract_fullpath = cli_gentemp(extract_dir);
if (NULL == extract_fullpath) {
cli_dbgmsg("RAR: Memory error allocating filename for extracted file.");
status = CL_EMEM;

42
libclamav/str.c
Unescape View File

@ -502,6 +502,48 @@ char *cli_strndup(const char *s, size_t n)
}
#endif
#if !defined(HAVE_STRNSTR) || defined(HAVE_STRNI)
/*
* @brief Find the first occurrence of find in s.
*
* The search is limited to the first slen characters of s.
*
* Copyright (c) 2001 Mike Barcroft <mike@FreeBSD.org>
* Copyright (c) 1990, 1993
* The Regents of the University of California. All rights reserved.
*
* This code is derived from software contributed to Berkeley by
* Chris Torek.
*
* Copyright (c) 1990 The Regents of the University of California.
* All rights reserved.
*
* @param s haystack
* @param find needle
* @param slen haystack length
* @return char* Address of the needle, if found, else NULL.
*/
char *cli_strnstr(const char *s, const char *find, size_t slen)
{
char c, sc;
size_t len;
if ((c = *find++) != '\0') {
len = strlen(find);
do {
do {
if (slen-- < 1 || (sc = *s++) == '\0')
return (NULL);
} while (sc != c);
if (len > slen)
return (NULL);
} while (strncmp(s, find, len) != 0);
s--;
}
return ((char *)s);
}
#endif
size_t cli_strtokenize(char *buffer, const char delim, const size_t token_count,
const char **tokens)
{

12
libclamav/str.h
Unescape View File

@ -3,7 +3,7 @@
* Copyright (C) 2007-2013 Sourcefire, Inc.
*
* Authors: Tomasz Kojm, Nigel Horne, Török Edvin
*
*
* Acknowledgements: cli_strcasestr() contains a public domain code from:
* http://unixpapa.com/incnote/string.html
*
@ -52,6 +52,12 @@ char *cli_strndup(const char *s, size_t n);
size_t cli_strnlen(const char *s, size_t n);
#endif
#if defined(HAVE_STRNSTR) && !defined(HAVE_STRNI)
#define cli_strnstr strnstr
#else
char *cli_strnstr(const char *s, const char *find, size_t slen);
#endif
#include <stdio.h>
#define cli_nocase(val) tolower(val)
#define cli_nocasei(val) toupper(val)
@ -95,10 +101,10 @@ size_t cli_strlcat(char *dst, const char *src, size_t sz); /* libclamav/strlcat.
/**
* @brief Get the file basename including extension from a file path.
*
*
* Caller is responsible for freeing filebase.
* An empty string will be returned if the caller inputs a directory with a trailing slash (no file).
*
*
* @param filepath The filepath in question.
* @param[out] filebase An allocated string containing the file basename.
* @return cl_error_t CL_SUCCESS, CL_EARG, CL_EFORMAT, or CL_EMEM

1
m4/reorganization/code_checks/functions.m4
Unescape View File

@ -5,6 +5,7 @@ AC_SEARCH_LIBS([gethostent],[nsl], [(LIBS="$LIBS -lnsl"; CLAMAV_MILTER_LIBS="$CL
AC_CHECK_FUNCS_ONCE([poll setsid memcpy snprintf vsnprintf strerror_r strlcpy strlcat strcasestr inet_ntop setgroups initgroups ctime_r mkstemp mallinfo madvise getnameinfo])
AC_CHECK_FUNCS([strndup])
AC_CHECK_FUNCS([strnlen])
AC_CHECK_FUNCS([strnstr])
AC_FUNC_FSEEKO
dnl Check if anon maps are available, check if we can determine the page size

106
unit_tests/check_clamav.c
Unescape View File

@ -23,6 +23,7 @@
#include "../libclamav/version.h"
#include "../libclamav/dsig.h"
#include "../libclamav/fpu.h"
#include "../platform.h"
#include "checks.h"
static int fpu_words = FPU_ENDIAN_INITME;
@ -882,11 +883,107 @@ START_TEST(test_sha256)
}
END_TEST
START_TEST(test_sanitize_path)
{
char *sanitized = NULL;
const char *unsanitized = NULL;
unsanitized = "";
sanitized = cli_sanitize_filepath(unsanitized, strlen(unsanitized));
fail_if(NULL != sanitized, "sanitize_path: Empty path test failed");
unsanitized = NULL;
sanitized = cli_sanitize_filepath(unsanitized, 0);
fail_if(NULL != sanitized, "sanitize_path: NULL path #1 test failed");
unsanitized = NULL;
sanitized = cli_sanitize_filepath(unsanitized, 50);
fail_if(NULL != sanitized, "sanitize_path: NULL path #2 test failed");
unsanitized = "badlen";
sanitized = cli_sanitize_filepath(unsanitized, 0);
fail_if(NULL != sanitized, "sanitize_path: Zero/bad path length test failed");
unsanitized = ".." PATHSEP;
sanitized = cli_sanitize_filepath(unsanitized, strlen(unsanitized));
fail_if(NULL != sanitized, "sanitize_path: sanitized path should have been NULL");
unsanitized = "." PATHSEP;
sanitized = cli_sanitize_filepath(unsanitized, strlen(unsanitized));
fail_if(NULL != sanitized, "sanitize_path: sanitized path should have been NULL (2)");
unsanitized = PATHSEP;
sanitized = cli_sanitize_filepath(unsanitized, strlen(unsanitized));
fail_if(NULL != sanitized, "sanitize_path: sanitized path should have been NULL (3)");
unsanitized = ".." PATHSEP "relative_bad_1";
sanitized = cli_sanitize_filepath(unsanitized, strlen(unsanitized));
fail_if(NULL == sanitized);
fail_unless(!strcmp(sanitized, "relative_bad_1"), "sanitize_path: bad relative path test #1 failed");
free(sanitized);
unsanitized = "relative" PATHSEP ".." PATHSEP "good";
sanitized = cli_sanitize_filepath(unsanitized, strlen(unsanitized));
fail_if(NULL == sanitized);
fail_unless(!strcmp(sanitized, "relative" PATHSEP ".." PATHSEP "good"), "sanitize_path: good relative path test failed");
free(sanitized);
unsanitized = "relative" PATHSEP ".." PATHSEP ".." PATHSEP "bad_2";
sanitized = cli_sanitize_filepath(unsanitized, strlen(unsanitized));
fail_if(NULL == sanitized);
fail_unless(!strcmp(sanitized, "relative" PATHSEP ".." PATHSEP "bad_2"), "sanitize_path: bad relative path test failed");
free(sanitized);
unsanitized = "relative" PATHSEP "." PATHSEP ".." PATHSEP ".." PATHSEP "bad_current";
sanitized = cli_sanitize_filepath(unsanitized, strlen(unsanitized));
fail_if(NULL == sanitized);
fail_unless(!strcmp(sanitized, "relative" PATHSEP ".." PATHSEP "bad_current"), "sanitize_path: bad relative current path test failed");
free(sanitized);
unsanitized = "relative/../../bad_win_posix_path";
sanitized = cli_sanitize_filepath(unsanitized, strlen(unsanitized));
fail_if(NULL == sanitized);
fail_unless(!strcmp(sanitized, "relative/../bad_win_posix_path"), "sanitize_path: bad relative win posix path test failed");
free(sanitized);
unsanitized = "" PATHSEP "absolute" PATHSEP ".." PATHSEP ".." PATHSEP "bad";
sanitized = cli_sanitize_filepath(unsanitized, strlen(unsanitized));
fail_if(NULL == sanitized);
fail_unless(!strcmp(sanitized, "absolute" PATHSEP ".." PATHSEP "bad"), "sanitize_path: bad absolute path test failed");
free(sanitized);
unsanitized = "" PATHSEP "absolute" PATHSEP ".." PATHSEP "good";
sanitized = cli_sanitize_filepath(unsanitized, strlen(unsanitized));
fail_if(NULL == sanitized);
fail_unless(!strcmp(sanitized, "absolute" PATHSEP ".." PATHSEP "good"), "sanitize_path: good absolute path test failed");
free(sanitized);
unsanitized = "relative" PATHSEP "normal";
sanitized = cli_sanitize_filepath(unsanitized, strlen(unsanitized));
fail_if(NULL == sanitized);
fail_unless(!strcmp(sanitized, "relative" PATHSEP "normal"), "sanitize_path: relative normal path test failed");
free(sanitized);
unsanitized = "relative" PATHSEP PATHSEP "doublesep";
sanitized = cli_sanitize_filepath(unsanitized, strlen(unsanitized));
fail_if(NULL == sanitized);
fail_unless(!strcmp(sanitized, "relative" PATHSEP "doublesep"), "sanitize_path: relative double sep path test failed");
free(sanitized);
unsanitized = "relative" PATHSEP "shortname" PATHSEP "1";
sanitized = cli_sanitize_filepath(unsanitized, strlen(unsanitized));
fail_if(NULL == sanitized);
fail_unless(!strcmp(sanitized, "relative" PATHSEP "shortname" PATHSEP "1"), "sanitize_path: relative short name path test failed");
free(sanitized);
}
END_TEST
static Suite *test_cli_suite(void)
{
Suite *s = suite_create("cli");
TCase *tc_cli_others = tcase_create("byteorder_macros");
TCase *tc_cli_dsig = tcase_create("digital signatures");
Suite *s = suite_create("cli");
TCase *tc_cli_others = tcase_create("byteorder_macros");
TCase *tc_cli_dsig = tcase_create("digital signatures");
TCase *tc_cli_assorted = tcase_create("assorted functions");
suite_add_tcase(s, tc_cli_others);
tcase_add_checked_fixture(tc_cli_others, data_setup, data_teardown);
@ -898,6 +995,9 @@ static Suite *test_cli_suite(void)
tcase_add_loop_test(tc_cli_dsig, test_cli_dsig, 0, dsig_tests_cnt);
tcase_add_test(tc_cli_dsig, test_sha256);
suite_add_tcase(s, tc_cli_assorted);
tcase_add_test(tc_cli_assorted, test_sanitize_path);
return s;
}
#endif /* CHECK_HAVE_LOOPS */

6
win32/clamav-config.h
Unescape View File

@ -361,6 +361,12 @@
/* Define to 1 if you have the `strlcpy' function. */
/* #undef HAVE_STRLCPY */
/* Define to 1 if you have the `strndup' function. */
/* #undef HAVE_STRNDUP */
/* Define to 1 if you have the `strnstr' function. */
/* #undef HAVE_STRNSTR */
/* Define to 1 if sysconf(_SC_PAGESIZE) is available */
/* #undef HAVE_SYSCONF_SC_PAGESIZE */

6
win32/update-win32.pl
Unescape View File

@ -8,7 +8,7 @@ use File::Temp 'tempfile';
#########################################################
# HACK HERE HACK HERE HACK HERE HACK HERE HACK HERE #
# HACK HERE HACK HERE HACK HERE HACK HERE HACK HERE #
#########################################################
use constant DEBUG => 0;
@ -135,6 +135,8 @@ my %CONF = (
'HAVE_STRING_H' => '1',
'HAVE_STRLCAT' => -1,
'HAVE_STRLCPY' => -1,
'HAVE_STRNDUP' => -1,
'HAVE_STRNSTR' => -1,
'HAVE_SYSCONF_SC_PAGESIZE' => -1,
'HAVE_SYSTEM_TOMMATH' => -1,
'HAVE_SYS_DL_H' => -1,
@ -257,7 +259,7 @@ my @PROJECTS = (
);
###########################################################
# STOP HACKING HERE STOP HACKING HERE STOP HACKING HERE #
# STOP HACKING HERE STOP HACKING HERE STOP HACKING HERE #
###########################################################

Write Preview
Loading…
Cancel
Save