cvs: php4 /ext/standard basic_functions.c basic_functions.h file.c file.h /ext/swf swf.c /main SAPI.c rfc1867.c rfc1867.h

zeev Sat Sep 9 04:41:14 2000 EDT Modified files: /php4/ext/standard basic_functions.c basic_functions.h file.c file.h /php4/ext/swf swf.c /php4/main SAPI.c rfc1867.c rfc1867.h Log: 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. Index: php4/ext/standard/basic_functions.c diff -u php4/ext/standard/basic_functions.c:1.252 php4/ext/standard/basic_functions.c:1.253 --- php4/ext/standard/basic_functions.c:1.252 Fri Sep 8 15:31:21 2000 +++ php4/ext/standard/basic_functions.c Sat Sep 9 04:41:14 2000 @@ -17,7 +17,7 @@ +----------------------------------------------------------------------+ */ -/* $Id: basic_functions.c,v 1.252 2000/09/08 22:31:21 zeev Exp $ */ +/* $Id: basic_functions.c,v 1.253 2000/09/09 11:41:14 zeev Exp $ */ #include "php.h" #include "php_main.h" @@ -357,6 +357,7 @@ 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(move_uploaded_file) { zval **path, **new_path; + zend_bool successful=0; SLS_FETCH(); if (!SG(rfc1867_uploaded_files)) { @@ -2248,18 +2250,20 @@ 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); } Index: php4/ext/standard/basic_functions.h diff -u php4/ext/standard/basic_functions.h:1.64 php4/ext/standard/basic_functions.h:1.65 --- php4/ext/standard/basic_functions.h:1.64 Fri Sep 8 15:26:07 2000 +++ php4/ext/standard/basic_functions.h Sat Sep 9 04:41:14 2000 @@ -17,7 +17,7 @@ +----------------------------------------------------------------------+ */ -/* $Id: basic_functions.h,v 1.64 2000/09/08 22:26:07 zeev Exp $ */ +/* $Id: basic_functions.h,v 1.65 2000/09/09 11:41:14 zeev Exp $ */ #ifndef BASIC_FUNCTIONS_H #define BASIC_FUNCTIONS_H @@ -110,6 +110,7 @@ PHP_FUNCTION(unregister_tick_function); PHP_FUNCTION(is_uploaded_file); +PHP_FUNCTION(move_uploaded_file); /* From the INI parser */ PHP_FUNCTION(parse_ini_file); Index: php4/ext/standard/file.c diff -u php4/ext/standard/file.c:1.110 php4/ext/standard/file.c:1.111 --- php4/ext/standard/file.c:1.110 Fri Sep 8 18:15:50 2000 +++ php4/ext/standard/file.c Sat Sep 9 04:41:14 2000 @@ -13,11 +13,14 @@ | license@php.net so we can mail you a copy immediately. | +----------------------------------------------------------------------+ | Authors: Rasmus Lerdorf <rasmus@lerdorf.on.ca> | + | Stig Bakken <ssb@fast.no> | + | Andi Gutmans <andi@zend.com> | + | Zeev Suraski <zeev@zend.com> | | PHP 4.0 patches by Thies C. Arntzen (thies@digicol.de) | +----------------------------------------------------------------------+ */ -/* $Id: file.c,v 1.110 2000/09/09 01:15:50 ssb Exp $ */ +/* $Id: file.c,v 1.111 2000/09/09 11:41:14 zeev Exp $ */ /* Synced with php 3.0 revision 1.218 1999-06-16 [ssb] */ @@ -107,16 +110,16 @@ 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 */ -#ifndef HAVE_TEMPNAM +/* Loosely based on a tempnam() implementation by UCLA */ + /* * Copyright (c) 1988, 1993 * The Regents of the University of California. All rights reserved. @@ -161,53 +164,108 @@ # define MAXPATHLEN 255 # endif #endif + +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; +} -char *tempnam(const char *dir, const char *pfx) +/* 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 (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 (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 ((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_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_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 @@ 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 @@ { 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 @@ 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); } /* }}} */ Index: php4/ext/standard/file.h diff -u php4/ext/standard/file.h:1.26 php4/ext/standard/file.h:1.27 --- php4/ext/standard/file.h:1.26 Fri Sep 8 15:31:21 2000 +++ php4/ext/standard/file.h Sat Sep 9 04:41:14 2000 @@ -16,7 +16,7 @@ +----------------------------------------------------------------------+ */ -/* $Id: file.h,v 1.26 2000/09/08 22:31:21 zeev Exp $ */ +/* $Id: file.h,v 1.27 2000/09/09 11:41:14 zeev Exp $ */ /* Synced with php 3.0 revision 1.30 1999-06-16 [ssb] */ @@ -70,7 +70,7 @@ 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 */ Index: php4/ext/swf/swf.c diff -u php4/ext/swf/swf.c:1.21 php4/ext/swf/swf.c:1.22 --- php4/ext/swf/swf.c:1.21 Tue Aug 15 10:30:41 2000 +++ php4/ext/swf/swf.c Sat Sep 9 04:41:14 2000 @@ -16,7 +16,7 @@ +----------------------------------------------------------------------+ */ -/* $Id: swf.c,v 1.21 2000/08/15 17:30:41 sterling Exp $ */ +/* $Id: swf.c,v 1.22 2000/09/09 11:41:14 zeev Exp $ */ #include "php.h" @@ -164,6 +164,7 @@ { 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 @@ 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 @@ } #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; Index: php4/main/SAPI.c diff -u php4/main/SAPI.c:1.90 php4/main/SAPI.c:1.91 --- php4/main/SAPI.c:1.90 Fri Sep 8 15:31:21 2000 +++ php4/main/SAPI.c Sat Sep 9 04:41:14 2000 @@ -327,8 +327,7 @@ 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); } } Index: php4/main/rfc1867.c diff -u php4/main/rfc1867.c:1.48 php4/main/rfc1867.c:1.49 --- php4/main/rfc1867.c:1.48 Fri Sep 8 14:56:47 2000 +++ php4/main/rfc1867.c Sat Sep 9 04:41:14 2000 @@ -15,12 +15,11 @@ | Authors: Rasmus Lerdorf <rasmus@php.net> | +----------------------------------------------------------------------+ */ -/* $Id: rfc1867.c,v 1.48 2000/09/08 21:56:47 zeev Exp $ */ +/* $Id: rfc1867.c,v 1.49 2000/09/09 11:41:14 zeev Exp $ */ #include <stdio.h> #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 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 @@ 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 @@ 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 @@ 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; Index: php4/main/rfc1867.h diff -u php4/main/rfc1867.h:1.4 php4/main/rfc1867.h:1.5 --- php4/main/rfc1867.h:1.4 Sun Jul 2 16:46:50 2000 +++ php4/main/rfc1867.h Sat Sep 9 04:41:14 2000 @@ -10,4 +10,6 @@ #define FILE_UPLOAD_INPUT_BUFFER_SIZE 8192 +void destroy_uploaded_files_hash(SLS_D); + #endif /* RFC1867_H */