From 6c4cb4c0791fa3a0c115789b4d22ecf675438ecb Mon Sep 17 00:00:00 2001 From: Zeev Suraski Date: Sat, 9 Sep 2000 11:41:14 +0000 Subject: [PATCH] Security related updates: - Introduce php_open_temporary_file(), in place of tempnam(). Still needs testing under UNIX (mkstemp()), works reliably under Windows now. - Reimplement the mechanism for unlinking uploaded files at the end of the request (was it ever tested?). Files moved with move_uploaded_file() will not be unlink()'d again, to avoid (albeit very unlikely) race conditions. --- ext/standard/basic_functions.c | 20 ++-- ext/standard/basic_functions.h | 1 + ext/standard/file.c | 164 +++++++++++++++++++++------------ ext/standard/file.h | 2 +- ext/swf/swf.c | 21 ++++- main/SAPI.c | 3 +- main/rfc1867.c | 66 +++++++++---- main/rfc1867.h | 2 + 8 files changed, 187 insertions(+), 92 deletions(-) diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c index 7add16113f..1251b46acf 100644 --- a/ext/standard/basic_functions.c +++ b/ext/standard/basic_functions.c @@ -357,6 +357,7 @@ function_entry basic_functions[] = { PHP_FE(parse_ini_file, NULL) PHP_FE(is_uploaded_file, NULL) + PHP_FE(move_uploaded_file, NULL) /* functions from reg.c */ PHP_FE(ereg, third_argument_force_ref) @@ -2232,6 +2233,7 @@ PHP_FUNCTION(is_uploaded_file) PHP_FUNCTION(move_uploaded_file) { zval **path, **new_path; + zend_bool successful=0; SLS_FETCH(); if (!SG(rfc1867_uploaded_files)) { @@ -2248,18 +2250,20 @@ PHP_FUNCTION(move_uploaded_file) RETURN_FALSE; } + V_UNLINK(Z_STRVAL_PP(new_path)); if (rename(Z_STRVAL_PP(path), Z_STRVAL_PP(new_path))==0) { - RETURN_TRUE; + successful=1; + } else if (php_copy_file(Z_STRVAL_PP(path), Z_STRVAL_PP(new_path))==SUCCESS) { + V_UNLINK(Z_STRVAL_PP(path)); + successful=1; } - if (php_copy_file(Z_STRVAL_PP(path), Z_STRVAL_PP(new_path))==SUCCESS) { - unlink(Z_STRVAL_PP(path)); - RETURN_TRUE; + if (successful) { + zend_hash_del(SG(rfc1867_uploaded_files), Z_STRVAL_PP(path), Z_STRLEN_PP(path)+1); + } else { + php_error(E_WARNING, "Unable to move '%s' to '%s'", Z_STRVAL_PP(path), Z_STRVAL_PP(new_path)); } - - php_error(E_WARNING, "Unable to move '%s' to '%s'", Z_STRVAL_PP(path), Z_STRVAL_PP(new_path)); - - RETURN_FALSE; + RETURN_BOOL(successful); } diff --git a/ext/standard/basic_functions.h b/ext/standard/basic_functions.h index 6d15602f73..5151d24ac6 100644 --- a/ext/standard/basic_functions.h +++ b/ext/standard/basic_functions.h @@ -110,6 +110,7 @@ PHP_FUNCTION(register_tick_function); PHP_FUNCTION(unregister_tick_function); PHP_FUNCTION(is_uploaded_file); +PHP_FUNCTION(move_uploaded_file); /* From the INI parser */ PHP_FUNCTION(parse_ini_file); diff --git a/ext/standard/file.c b/ext/standard/file.c index a4db2dc274..d8b98228ef 100644 --- a/ext/standard/file.c +++ b/ext/standard/file.c @@ -13,6 +13,9 @@ | license@php.net so we can mail you a copy immediately. | +----------------------------------------------------------------------+ | Authors: Rasmus Lerdorf | + | Stig Bakken | + | Andi Gutmans | + | Zeev Suraski | | PHP 4.0 patches by Thies C. Arntzen (thies@digicol.de) | +----------------------------------------------------------------------+ */ @@ -107,16 +110,16 @@ php_file_globals file_globals; static void _file_fopen_dtor(FILE *pipe); static void _file_popen_dtor(FILE *pipe); static void _file_socket_dtor(int *sock); -static void _file_upload_dtor(char *file); /* sharing globals is *evil* */ -static int le_fopen,le_popen, le_socket, le_uploads; +static int le_fopen, le_popen, le_socket; /* }}} */ -/* {{{ tempnam */ +/* {{{ php_open_temporary_file */ + +/* Loosely based on a tempnam() implementation by UCLA */ -#ifndef HAVE_TEMPNAM /* * Copyright (c) 1988, 1993 * The Regents of the University of California. All rights reserved. @@ -162,52 +165,107 @@ static int le_fopen,le_popen, le_socket, le_uploads; # endif #endif -char *tempnam(const char *dir, const char *pfx) +static FILE *php_do_open_temporary_file(char *path, const char *pfx, char **opened_path_p) +{ + char *trailing_slash; + FILE *fp; + char *opened_path; + + if (!path) { + return NULL; + } + + if (!(opened_path = emalloc(MAXPATHLEN))) { + return NULL; + } + + if (*path+strlen(path)-1 == '/') { + trailing_slash = ""; + } else { + trailing_slash = "/"; + } + + (void)snprintf(opened_path, MAXPATHLEN, "%s%s%sXXXXXX", path, trailing_slash, pfx); + +#ifdef PHP_WIN32 + if (GetTempFileName(path, pfx, 0, opened_path)) { + fp = V_FOPEN(opened_path, "wb"); + } else { + fp = NULL; + } +#elif defined(HAVE_MKSTEMP) + fd = mkstemp(opened_path); + if (fd==-1) { + fp = NULL; + } else { + fp = fdopen(fd, "wb"); + } +#else + if (mktemp(opened_path)) { + fp = V_FOPEN(opened_path, "wb"); + } else { + fp = NULL; + } +#endif + if (!fp || !opened_path_p) { + efree(opened_path); + } else { + *opened_path_p = opened_path; + } + return fp; +} + +/* Unlike tempnam(), the supplied dir argument takes precedence + * over the TMPDIR environment variable + * This function should do its best to return a file pointer to a newly created + * unique file, on every platform. + */ +FILE *php_open_temporary_file(const char *dir, const char *pfx, char **opened_path_p) { - int save_errno; - char *f, *name; static char path_tmp[] = "/tmp"; + FILE *fp; - if (!(name = emalloc(MAXPATHLEN))) { - return(NULL); - } if (!pfx) { pfx = "tmp."; } - - if (f = getenv("TMPDIR")) { - (void)snprintf(name, MAXPATHLEN, "%s%s%sXXXXXX", f, - *(f + strlen(f) - 1) == '/'? "": "/", pfx); - if (f = mktemp(name)) - return(f); + + if (opened_path_p) { + *opened_path_p = NULL; + } + + if ((fp=php_do_open_temporary_file((char *) dir, pfx, opened_path_p))) { + return fp; + } + + if ((fp=php_do_open_temporary_file(getenv("TMPDIR"), pfx, opened_path_p))) { + return fp; + } +#if PHP_WIN32 + { + char *TempPath; + + TempPath = (char *) emalloc(MAXPATHLEN); + if (GetTempPath(MAXPATHLEN, TempPath)) { + fp = php_do_open_temporary_file(TempPath, pfx, opened_path_p); + } + efree(TempPath); + return fp; + } +#else + if ((fp=php_do_open_temporary_file(P_tmpdir, pfx, opened_path_p))) { + return fp; } - - if (f = (char *)dir) { - (void)snprintf(name, MAXPATHLEN, "%s%s%sXXXXXX", f, - *(f + strlen(f) - 1) == '/'? "": "/", pfx); - if (f = mktemp(name)) - return(f); - } - - f = P_tmpdir; - (void)snprintf(name, MAXPATHLEN, "%s%sXXXXXX", f, pfx); - if (f = mktemp(name)) - return(f); - - f = path_tmp; - (void)snprintf(name, MAXPATHLEN, "%s%sXXXXXX", f, pfx); - if (f = mktemp(name)) - return(f); - - save_errno = errno; - efree(name); - errno = save_errno; - return(NULL); -} + if ((fp=php_do_open_temporary_file(path_tmp, pfx, opened_path_p))) { + return fp; + } #endif + return NULL; +} + + /* }}} */ /* {{{ Module-Stuff */ @@ -228,12 +286,6 @@ static void _file_socket_dtor(int *sock) } -static void _file_upload_dtor(char *file) -{ - V_UNLINK(file); -} - - static void _file_fopen_dtor(FILE *fp) { fclose(fp); @@ -257,12 +309,6 @@ PHPAPI int php_file_le_socket(void) /* XXX doe we really want this???? */ } -PHPAPI int php_file_le_uploads(void) /* XXX doe we really want this???? */ -{ - return le_uploads; -} - - #ifdef ZTS static void php_file_init_globals(php_file_globals *file_globals) { @@ -276,7 +322,6 @@ PHP_MINIT_FUNCTION(file) le_fopen = register_list_destructors(_file_fopen_dtor, NULL); le_popen = register_list_destructors(_file_popen_dtor, NULL); le_socket = register_list_destructors(_file_socket_dtor, NULL); - le_uploads = register_list_destructors(_file_upload_dtor, NULL); #ifdef ZTS file_globals_id = ts_allocate_id(sizeof(php_file_globals), (ts_allocate_ctor) php_file_init_globals, NULL); @@ -581,8 +626,9 @@ PHP_FUNCTION(tempnam) { pval **arg1, **arg2; char *d; - char *t; + char *opened_path; char p[64]; + FILE *fp; if (ARG_COUNT(ht) != 2 || zend_get_parameters_ex(2, &arg1, &arg2) == FAILURE) { WRONG_PARAM_COUNT; @@ -592,12 +638,14 @@ PHP_FUNCTION(tempnam) d = estrndup((*arg1)->value.str.val,(*arg1)->value.str.len); strlcpy(p,(*arg2)->value.str.val,sizeof(p)); - t = tempnam(d,p); - efree(d); - if(!t) { - RETURN_FALSE; + + if ((fp = php_open_temporary_file(d, p, &opened_path))) { + fclose(fp); + RETVAL_STRING(opened_path, 0); + } else { + RETVAL_FALSE; } - RETURN_STRING(t,1); + efree(d); } /* }}} */ diff --git a/ext/standard/file.h b/ext/standard/file.h index 0107631d1d..a7e6201569 100644 --- a/ext/standard/file.h +++ b/ext/standard/file.h @@ -70,7 +70,7 @@ PHPAPI int php_set_sock_blocking(int socketd, int block); PHPAPI int php_file_le_fopen(void); PHPAPI int php_file_le_popen(void); PHPAPI int php_file_le_socket(void); -PHPAPI int php_file_le_uploads(void); PHPAPI int php_copy_file(char *src, char *dest); +PHPAPI FILE *php_open_temporary_file(const char *dir, const char *pfx, char **opened_path_p); #endif /* FILE_H */ diff --git a/ext/swf/swf.c b/ext/swf/swf.c index 189d21debf..6c7f2a3157 100644 --- a/ext/swf/swf.c +++ b/ext/swf/swf.c @@ -164,6 +164,7 @@ PHP_FUNCTION(swf_openfile) { zval **name, **sizeX, **sizeY, **frameRate, **r, **g, **b; char *na, *tmpna; + zend_bool free_na; SWFLS_FETCH(); if (ZEND_NUM_ARGS() != 7 || @@ -183,9 +184,16 @@ PHP_FUNCTION(swf_openfile) tmpna = Z_STRVAL_PP(name); if (strcasecmp("php://stdout", tmpna) == 0) { - na = tempnam(NULL, "php_swf_stdout"); + FILE *fp; + + fp = php_open_temporary_file(NULL, "php_swf_stdout", &na); + if (!fp) { + free_na = 0; + RETURN_FALSE; + } unlink((const char *)na); - + fclose(fp); + free_na = 1; SWFG(use_file) = 0; } else { na = tmpna; @@ -193,9 +201,16 @@ PHP_FUNCTION(swf_openfile) } #ifdef VIRTUAL_DIR - if (virtual_filepath(na, &na)) { + if (virtual_filepath(na, &tmpna)) { + if (free_na) { + efree(na); + } return; } + if (free_na) { + efree(na); + } + na = tmpna; #endif if (!SWFG(use_file)) SWFG(tmpfile_name) = na; diff --git a/main/SAPI.c b/main/SAPI.c index a24ca9aaf8..7853292383 100644 --- a/main/SAPI.c +++ b/main/SAPI.c @@ -327,8 +327,7 @@ SAPI_API void sapi_deactivate(SLS_D) sapi_module.deactivate(SLS_C); } if (SG(rfc1867_uploaded_files)) { - zend_hash_destroy(SG(rfc1867_uploaded_files)); - FREE_HASHTABLE(SG(rfc1867_uploaded_files)); + destroy_uploaded_files_hash(SLS_C); } } diff --git a/main/rfc1867.c b/main/rfc1867.c index 68af24557c..5d5d4fcf6f 100644 --- a/main/rfc1867.c +++ b/main/rfc1867.c @@ -20,7 +20,6 @@ #include #include "php.h" #include "ext/standard/php_standard.h" -#include "ext/standard/file.h" /* for php_file_le_uploads() */ #include "zend_globals.h" #include "php_globals.h" #include "php_variables.h" @@ -83,12 +82,32 @@ static void register_http_post_files_variable_ex(char *var, zval *val, zval *htt } +static void free_filename(char **filename) +{ + efree(*filename); +} + + +static int unlink_filename(char **filename) +{ + V_UNLINK(*filename); + return 0; +} + + +void destroy_uploaded_files_hash(SLS_D) +{ + zend_hash_apply(SG(rfc1867_uploaded_files), (apply_func_t) unlink_filename); + zend_hash_destroy(SG(rfc1867_uploaded_files)); + FREE_HASHTABLE(SG(rfc1867_uploaded_files)); +} + /* * Split raw mime stream up into appropriate components */ static void php_mime_split(char *buf, int cnt, char *boundary, zval *array_ptr SLS_DC) { - char *ptr, *loc, *loc2, *loc3, *s, *name, *filename, *u, *fn; + char *ptr, *loc, *loc2, *loc3, *s, *name, *filename, *u, *temp_filename; int len, state = 0, Done = 0, rem, urem; int eolsize; long bytes, max_file_size = 0; @@ -97,13 +116,15 @@ static void php_mime_split(char *buf, int cnt, char *boundary, zval *array_ptr S FILE *fp; int itype, is_arr_upload=0, arr_len=0; zval *http_post_files=NULL; + zend_bool upload_successful; + zend_bool magic_quotes_gpc; ELS_FETCH(); PLS_FETCH(); zend_hash_init(&PG(rfc1867_protected_variables), 5, NULL, NULL, 0); ALLOC_HASHTABLE(SG(rfc1867_uploaded_files)); - zend_hash_init(SG(rfc1867_uploaded_files), 5, NULL, NULL, 0); + zend_hash_init(SG(rfc1867_uploaded_files), 5, NULL, (dtor_func_t) free_filename, 0); ALLOC_ZVAL(http_post_files); array_init(http_post_files); @@ -327,36 +348,39 @@ static void php_mime_split(char *buf, int cnt, char *boundary, zval *array_ptr S SAFE_RETURN; } bytes = 0; - fn = tempnam(PG(upload_tmp_dir), "php"); + + fp = php_open_temporary_file(PG(upload_tmp_dir), "php", &temp_filename); + if (!fp) { + php_error(E_WARNING, "File upload error - unable to create a temporary file"); + SAFE_RETURN; + } if ((loc - ptr - 4) > PG(upload_max_filesize)) { php_error(E_WARNING, "Max file size of %ld bytes exceeded - file [%s] not saved", PG(upload_max_filesize),namebuf); - fn = "none"; + upload_successful = 0; } else if (max_file_size && ((loc - ptr - 4) > max_file_size)) { php_error(E_WARNING, "Max file size exceeded - file [%s] not saved", namebuf); - fn = "none"; + upload_successful = 0; } else if ((loc - ptr - 4) <= 0) { - fn = "none"; + upload_successful = 0; } else { - fp = V_FOPEN(fn, "wb"); - if (!fp) { - php_error(E_WARNING, "File Upload Error - Unable to open temporary file [%s]", fn); - SAFE_RETURN; - } bytes = fwrite(ptr, 1, loc - ptr - 4, fp); - fclose(fp); - zend_list_insert(fn,php_file_le_uploads()); /* Tell PHP about the file so the destructor can unlink it later */ if (bytes < (loc - ptr - 4)) { php_error(E_WARNING, "Only %d bytes were written, expected to write %ld", bytes, loc - ptr - 4); } + upload_successful = 1; } + fclose(fp); add_protected_variable(namebuf PLS_CC); - safe_php_register_variable(namebuf, fn, NULL, 1 ELS_CC PLS_CC); - { - int dummy=1; - - zend_hash_add(SG(rfc1867_uploaded_files), namebuf, strlen(namebuf)+1, &dummy, sizeof(int), NULL); + if (!upload_successful) { + efree(temp_filename); + temp_filename = "none"; + } else { + zend_hash_add(SG(rfc1867_uploaded_files), temp_filename, strlen(temp_filename)+1, &temp_filename, sizeof(char *), NULL); } + magic_quotes_gpc = PG(magic_quotes_gpc); + PG(magic_quotes_gpc) = 0; + safe_php_register_variable(namebuf, temp_filename, NULL, 1 ELS_CC PLS_CC); /* Add $foo[tmp_name] */ if(is_arr_upload) { sprintf(lbuf, "%s[tmp_name][%s]", abuf, arr_index); @@ -364,7 +388,9 @@ static void php_mime_split(char *buf, int cnt, char *boundary, zval *array_ptr S sprintf(lbuf, "%s[tmp_name]", namebuf); } add_protected_variable(lbuf PLS_CC); - register_http_post_files_variable(lbuf, fn, http_post_files, 1 ELS_CC PLS_CC); + register_http_post_files_variable(lbuf, temp_filename, http_post_files, 1 ELS_CC PLS_CC); + PG(magic_quotes_gpc) = magic_quotes_gpc; + { zval file_size; diff --git a/main/rfc1867.h b/main/rfc1867.h index 7e84022fe8..1bcba038ea 100644 --- a/main/rfc1867.h +++ b/main/rfc1867.h @@ -10,4 +10,6 @@ SAPI_POST_HANDLER_FUNC(rfc1867_post_handler); #define FILE_UPLOAD_INPUT_BUFFER_SIZE 8192 +void destroy_uploaded_files_hash(SLS_D); + #endif /* RFC1867_H */ -- 2.40.0