Edit report at https://bugs.php.net/bug.php?id=46439&edit=1
ID: 46439
Comment by: gmblar+php at gmail dot com
Reported by: tom at punkave dot com
Summary: file upload implementation is flawed
Status: Open
Type: Bug
Package: cURL related
Operating System: *
PHP Version: 5.*, 6CVS (2009-01-21)
Block user comment: N
Private report: N
New Comment:
There is no function to escape the "@" in the CURLOPT_POSTFIELDS array and in
this example
its not possible to remove or replace the "@".
$curl = curl_init();
curl_setopt_array($curl, array(
CURLOPT_URL => 'http://www.example.com/',
CURLOPT_RETURNTRANSFER => true,
CURLOPT_POSTFIELDS => array(
'username' => 'foobar',
// Users may have strange passwords. Should be transfered as text.
'password' => '@/etc/hosts',
// Upload image.
'picture' => '@/var/www/avatars/foobar.jpg'
)
));
curl_exec($curl);
curl_close($curl);
My suggestion is to escape the password in this escape with \@ and then thread
as text.
Previous Comments:
------------------------------------------------------------------------
[2009-05-03 21:04:44] paj...@php.net
tbd
------------------------------------------------------------------------
[2009-01-21 20:08:07] tom at punkave dot com
htmlencode() won't escape @. Neither will htmlentities(). it's a security bug
that no amount of reasonable prudence on the part of programmers who haven't
read this particular bug report will address. And there is no reason why
programmers would expect that filtering input would be necessary when they are
passing individual fields to a function that ought to be ready to escape them
(and in fact does, apart from the leading @ thing).
The documentation needs to be fixed at a minimum. It would be a much better
idea to get rid of the broken behavior. The @ prefix is a bad idea (what if I
want to pass @?) and with the current lack of documentation it's a security
hole.
This needs to be patched or at least documented.
------------------------------------------------------------------------
[2009-01-21 19:56:56] j...@php.net
It's security hole only if you don't filter the input..
------------------------------------------------------------------------
[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 https://bugs.php.net/bug.php?id=46439&edit=1
