ID: 46439
Updated by: j...@php.net
-Summary: file upload implementation is a security hole
Reported By: tom at punkave dot com
Status: Open
Bug Type: cURL related
Operating System: *
PHP Version: 5.*CVS,6CVS (2009-01-21)
New Comment:
It's security hole only if you don't filter the input..
Previous Comments:
------------------------------------------------------------------------
[2008-10-31 19:18:36] tom at punkave dot com
Description:
------------
PHP's cURL wrapper implements HTTP POST file uploads as follows:
curl_setopt($curl_handle, CURLOPT_POST, 1);
$args['file'] = '@/path/to/file';
curl_setopt($curl_handle, CURLOPT_POSTFIELDS, $args);
When ext/curl/interface.c sees that $args is an array and not a
query-encoded string, it switches to a branch that uses CURLOPT_HTTPPOST
rather than CURLOPT_POST. The code then checks for an '@' prefix in the
value of every field. When an '@' is spotted, that particular field is
treated as a file to be uploaded rather than a value to be sent as-is.
This implementation and the associated documentation have the following
problems which are best described together for clarity's sake:
1. The fact that passing an array of arguments will trigger
multipart/form-data is not documented. The documentation implies that
you can use a query-encoded string or an array interchangeably. While
most servers do accept multipart/form-data this is not a given. Also it
is frequently the less efficient of the two encodings when files are not
being uploaded.
2. When passing an array it is impossible to submit a form field value
that does start with @. This is a bug in the implementation.
3. The documentation makes no mention of the '@ prefix means the rest
of the value is a filename to be uploaded' issue. This is a serious
security problem. PHP pages that transship form submissions from one
site to another are being coded in ignorance of the fact that the '@'
prefix could be used by end users to send any readable file on the first
host to the second host. At a minimum, coders must check for and remove
any @ prefix from user-submitted fields.
A recommended solution:
1. The '@ prefix for files, arrays trigger multipart/form-data'
behavior should be controlled by a php.ini backwards compatibility
option, hopefully defaulting off in the future.
2. CURLOPT_HTTPPOST and CURLOPT_HTTPPOSTFIELDS should be explicitly
supported and documented as the correct way to do multipart/form_data,
and
3. Instead of an @ prefix in the values of fields,
CURLOPT_HTTPPOSTFILEFIELDS should be implemented to expressly pass an
hash of keys => filenames.
It would work like this:
// I want a file upload with cURL
curl_setopt($curl_handle, CURLOPT_HTTPPOST, 1);
// Pass the non-file fields
curl_setopt($curl_handle, CURLOPT_HTTPPOSTFIELDS,
array("name" => "Joe Smith"));
// Pass the file fields
curl_setopt($curl_handle, CURLOPT_HTTPPOSTFILEFIELDS,
array("file" => "/path/to/file"));
HTTPPOST is a terrible, confusing name for multipart/form_data, but
that's a cURL problem, not a PHP problem. (: With the above
implementation at the PHP level we would at least have a correct wrapper
for cURL on which friendlier classes could be correctly built.
------------------------------------------------------------------------
--
Edit this bug report at http://bugs.php.net/?id=46439&edit=1
