From b1bb4ca6d8777683b6a549fb61dba36759da26f4 Mon Sep 17 00:00:00 2001 From: Ray Satiro Date: Tue, 26 Jan 2016 23:23:15 +0100 Subject: [PATCH] curl: avoid local drive traversal when saving file (Windows) curl does not sanitize colons in a remote file name that is used as the local file name. This may lead to a vulnerability on systems where the colon is a special path character. Currently Windows/DOS is the only OS where this vulnerability applies. CVE-2016-0754 Bug: http://curl.haxx.se/docs/adv_20160127B.html Upstream-Status: Backport http://curl.haxx.se/CVE-2016-0754.patch CVE: CVE-2016-0754 Signed-off-by: Armin Kuster --- src/tool_cb_hdr.c | 40 ++++++------ src/tool_doswin.c | 174 ++++++++++++++++++++++++++++++++++++++++++++--------- src/tool_doswin.h | 2 +- src/tool_operate.c | 29 ++++++--- 4 files changed, 187 insertions(+), 58 deletions(-) Index: curl-7.40.0/src/tool_cb_hdr.c =================================================================== --- curl-7.40.0.orig/src/tool_cb_hdr.c +++ curl-7.40.0/src/tool_cb_hdr.c @@ -28,6 +28,7 @@ #include "curlx.h" #include "tool_cfgable.h" +#include "tool_doswin.h" #include "tool_msgs.h" #include "tool_cb_hdr.h" @@ -111,18 +112,24 @@ size_t tool_header_cb(void *ptr, size_t */ len = (ssize_t)cb - (p - str); filename = parse_filename(p, len); - if(filename) { - outs->filename = filename; - outs->alloc_filename = TRUE; - outs->is_cd_filename = TRUE; - outs->s_isreg = TRUE; - outs->fopened = FALSE; - outs->stream = NULL; - hdrcbdata->honor_cd_filename = FALSE; - break; - } - else + if(!filename) + return failure; + +#if defined(MSDOS) || defined(WIN32) + if(sanitize_file_name(&filename)) { + free(filename); return failure; + } +#endif /* MSDOS || WIN32 */ + + outs->filename = filename; + outs->alloc_filename = TRUE; + outs->is_cd_filename = TRUE; + outs->s_isreg = TRUE; + outs->fopened = FALSE; + outs->stream = NULL; + hdrcbdata->honor_cd_filename = FALSE; + break; } } @@ -178,15 +185,12 @@ static char *parse_filename(const char * } /* scan for the end letter and stop there */ - q = p; - while(*q) { - if(q[1] && (q[0] == '\\')) - q++; - else if(q[0] == stop) + for(q = p; *q; ++q) { + if(*q == stop) { + *q = '\0'; break; - q++; + } } - *q = '\0'; /* make sure the file name doesn't end in \r or \n */ q = strchr(p, '\r'); Index: curl-7.40.0/src/tool_doswin.c =================================================================== --- curl-7.40.0.orig/src/tool_doswin.c +++ curl-7.40.0/src/tool_doswin.c @@ -85,42 +85,106 @@ __pragma(warning(pop)) # include /* _use_lfn(f) prototype */ #endif -static const char *msdosify (const char *file_name); -static char *rename_if_dos_device_name (char *file_name); +static char *msdosify(const char *file_name); +static char *rename_if_dos_device_name(const char *file_name); -/* - * sanitize_dos_name: returns a newly allocated string holding a - * valid file name which will be a transformation of given argument - * in case this wasn't already a valid file name. - * - * This function takes ownership of given argument, free'ing it before - * returning. Caller is responsible of free'ing returned string. Upon - * out of memory condition function returns NULL. - */ -char *sanitize_dos_name(char *file_name) +/* +Sanitize *file_name. +Success: (CURLE_OK) *file_name points to a sanitized version of the original. + This function takes ownership of the original *file_name and frees it. +Failure: (!= CURLE_OK) *file_name is unchanged. +*/ +CURLcode sanitize_file_name(char **file_name) { - char new_name[PATH_MAX]; + size_t len; + char *p, *sanitized; + + /* Calculate the maximum length of a filename. + FILENAME_MAX is often the same as PATH_MAX, in other words it does not + discount the path information. PATH_MAX size is calculated based on: + */ + const size_t max_filename_len = PATH_MAX - 3 - 1; + + if(!file_name || !*file_name) + return CURLE_BAD_FUNCTION_ARGUMENT; + + len = strlen(*file_name); + + if(len >= max_filename_len) + len = max_filename_len - 1; + + sanitized = malloc(len + 1); - if(!file_name) - return NULL; + if(!sanitized) + return CURLE_OUT_OF_MEMORY; - if(strlen(file_name) >= PATH_MAX) - file_name[PATH_MAX-1] = '\0'; /* truncate it */ + strncpy(sanitized, *file_name, len); + sanitized[len] = '\0'; - strcpy(new_name, msdosify(file_name)); + for(p = sanitized; *p; ++p ) { + const char *banned; + if(1 <= *p && *p <= 31) { + *p = '_'; + continue; + } + for(banned = "|<>/\\\":?*"; *banned; ++banned) { + if(*p == *banned) { + *p = '_'; + break; + } + } + } - Curl_safefree(file_name); +#ifdef MSDOS + /* msdosify checks for more banned characters for MSDOS, however it allows + for some path information to pass through. since we are sanitizing only a + filename and cannot allow a path it's important this call be done in + addition to and not instead of the banned character check above. */ + p = msdosify(sanitized); + if(!p) { + free(sanitized); + return CURLE_BAD_FUNCTION_ARGUMENT; + } + sanitized = p; + len = strlen(sanitized); +#endif + + p = rename_if_dos_device_name(sanitized); + if(!p) { + free(sanitized); + return CURLE_BAD_FUNCTION_ARGUMENT; + } + sanitized = p; + len = strlen(sanitized); - return strdup(rename_if_dos_device_name(new_name)); + /* dos_device_name rename will rename a device name, possibly changing the + length. If the length is too long now we can't truncate it because we + could end up with a device name. In practice this shouldn't be a problem + because device names are short, but you never know. */ + if(len >= max_filename_len) { + free(sanitized); + return CURLE_BAD_FUNCTION_ARGUMENT; + } + + *file_name = sanitized; + return CURLE_OK; } -/* The following functions are taken with modification from the DJGPP - * port of tar 1.12. They use algorithms originally from DJTAR. */ +/* The functions msdosify, rename_if_dos_device_name and __crt0_glob_function + * were taken with modification from the DJGPP port of tar 1.12. They use + * algorithms originally from DJTAR. + */ -static const char *msdosify (const char *file_name) +/* +Extra sanitization MSDOS for file_name. +Returns a copy of file_name that is sanitized by MSDOS standards. +Warning: path information may pass through. For sanitizing a filename use +sanitize_file_name which calls this function after sanitizing path info. +*/ +static char *msdosify(const char *file_name) { - static char dos_name[PATH_MAX]; + char dos_name[PATH_MAX]; static const char illegal_chars_dos[] = ".+, ;=[]" /* illegal in DOS */ "|<>\\\":?*"; /* illegal in DOS & W95 */ static const char *illegal_chars_w95 = &illegal_chars_dos[8]; @@ -201,15 +265,20 @@ static const char *msdosify (const char } *d = '\0'; - return dos_name; + return strdup(dos_name); } -static char *rename_if_dos_device_name (char *file_name) +/* +Rename file_name if it's a representation of a device name. +Returns a copy of file_name, and the copy will have contents different from the +original if a device name was found. +*/ +static char *rename_if_dos_device_name(const char *file_name) { /* We could have a file whose name is a device on MS-DOS. Trying to * retrieve such a file would fail at best and wedge us at worst. We need * to rename such files. */ - char *base; + char *p, *base; struct_stat st_buf; char fname[PATH_MAX]; @@ -219,7 +288,7 @@ static char *rename_if_dos_device_name ( if(((stat(base, &st_buf)) == 0) && (S_ISCHR(st_buf.st_mode))) { size_t blen = strlen(base); - if(strlen(fname) >= PATH_MAX-1) { + if(strlen(fname) == PATH_MAX-1) { /* Make room for the '_' */ blen--; base[blen] = '\0'; @@ -227,9 +296,54 @@ static char *rename_if_dos_device_name ( /* Prepend a '_'. */ memmove(base + 1, base, blen + 1); base[0] = '_'; - strcpy(file_name, fname); } - return file_name; + + /* The above stat check does not identify devices for me in Windows 7. For + example a stat on COM1 returns a regular file S_IFREG. According to MSDN + stat doc that is the correct behavior, so I assume the above code is + legacy, maybe MSDOS or DJGPP specific? */ + + /* Rename devices. + Examples: CON => _CON, CON.EXT => CON_EXT, CON:ADS => CON_ADS */ + for(p = fname; p; p = (p == fname && fname != base ? base : NULL)) { + size_t p_len; + int x = (curl_strnequal(p, "CON", 3) || + curl_strnequal(p, "PRN", 3) || + curl_strnequal(p, "AUX", 3) || + curl_strnequal(p, "NUL", 3)) ? 3 : + (curl_strnequal(p, "CLOCK$", 6)) ? 6 : + (curl_strnequal(p, "COM", 3) || curl_strnequal(p, "LPT", 3)) ? + (('1' <= p[3] && p[3] <= '9') ? 4 : 3) : 0; + + if(!x) + continue; + + /* the devices may be accessible with an extension or ADS, for + example CON.AIR and CON:AIR both access console */ + if(p[x] == '.' || p[x] == ':') { + p[x] = '_'; + continue; + } + else if(p[x]) /* no match */ + continue; + + p_len = strlen(p); + + if(strlen(fname) == PATH_MAX-1) { + /* Make room for the '_' */ + p_len--; + p[p_len] = '\0'; + } + /* Prepend a '_'. */ + memmove(p + 1, p, p_len + 1); + p[0] = '_'; + + /* if fname was just modified then the basename pointer must be updated */ + if(p == fname) + base = basename(fname); + } + + return strdup(fname); } #if defined(MSDOS) && (defined(__DJGPP__) || defined(__GO32__)) Index: curl-7.40.0/src/tool_doswin.h =================================================================== --- curl-7.40.0.orig/src/tool_doswin.h +++ curl-7.40.0/src/tool_doswin.h @@ -25,7 +25,7 @@ #if defined(MSDOS) || defined(WIN32) -char *sanitize_dos_name(char *file_name); +CURLcode sanitize_file_name(char **filename); #if defined(MSDOS) && (defined(__DJGPP__) || defined(__GO32__)) Index: curl-7.40.0/src/tool_operate.c =================================================================== --- curl-7.40.0.orig/src/tool_operate.c +++ curl-7.40.0/src/tool_operate.c @@ -543,26 +543,37 @@ static CURLcode operate_do(struct Global result = get_url_file_name(&outfile, this_url); if(result) goto show_error; + +#if defined(MSDOS) || defined(WIN32) + result = sanitize_file_name(&outfile); + if(result) { + Curl_safefree(outfile); + goto show_error; + } +#endif /* MSDOS || WIN32 */ + if(!*outfile && !config->content_disposition) { helpf(global->errors, "Remote file name has no length!\n"); result = CURLE_WRITE_ERROR; goto quit_urls; } -#if defined(MSDOS) || defined(WIN32) - /* For DOS and WIN32, we do some major replacing of - bad characters in the file name before using it */ - outfile = sanitize_dos_name(outfile); - if(!outfile) { - result = CURLE_OUT_OF_MEMORY; - goto show_error; - } -#endif /* MSDOS || WIN32 */ } else if(urls) { /* fill '#1' ... '#9' terms from URL pattern */ char *storefile = outfile; result = glob_match_url(&outfile, storefile, urls); Curl_safefree(storefile); + +#if defined(MSDOS) || defined(WIN32) + if(!result) { + result = sanitize_file_name(&outfile); + if(result) { + Curl_safefree(outfile); + goto show_error; + } + } +#endif /* MSDOS || WIN32 */ + if(result) { /* bad globbing */ warnf(config, "bad output glob!\n");