Edit report at https://bugs.php.net/bug.php?id=54374&edit=1
ID: 54374
User updated by: lekensteyn at gmail dot com
Reported by: lekensteyn at gmail dot com
Summary: Insufficient validating of upload name leading to
corrupted $_FILES indices
Status: Open
-Type: Documentation Problem
+Type: Bug
Package: Variables related
Operating System: All
-PHP Version: 5.3.6
+PHP Version: 5.3.8
Block user comment: N
Private report: N
New Comment:
This is not a documentation bug and still exists in PHP 5.3.8.
Previous Comments:
------------------------------------------------------------------------
[2011-03-24 20:07:31] lekensteyn at gmail dot com
Description:
------------
SAPI: Apache 2 module (it should apply to other SAPI's which accepts uploads as
well)
OS: Debian 6 (it should apply to other OSes as well)
PHP: 5.3.6 (from source, test compile: ./configure --prefix=/tmp/diebug
--disable-all --with-apxs2=/tmp/diebug/bin/apxs --disable-cli)
Upload names with brackets ([ and ]) are created for creating arrays of files.
Any array index or variable name containing a bracket should be invalid.
The current implementation only checks whether more closing brackets are
detected than opening brackets.
Related files
http://lxr.php.net/opengrok/xref/PHP_5_3/main/rfc1867.c#990
http://lxr.php.net/opengrok/xref/PHP_TRUNK/main/rfc1867.c#920
Relevant code:
--snip--
/* New Rule: never repair potential malicious user input */
if (!skip_upload) {
long c = 0;
tmp = param;
while (*tmp) {
if (*tmp == '[') {
c++;
} else if (*tmp == ']') {
c--;
if (tmp[1] && tmp[1] != '[') {
skip_upload = 1;
break;
}
}
if (c < 0) {
skip_upload = 1;
break;
}
tmp++;
}
}
--snip--
So names like `test]` and `test[]]` are invalid, but names like `test[` pass
this test.
Now it gets worse, the upload is accepted and without checking the name, and
registered:
--snip--
if (is_arr_upload) {
snprintf(lbuf, llen, "%s[name][%s]", abuf, array_index);
} else {
snprintf(lbuf, llen, "%s[name]", param);
}
if (s && s > filename) {
register_http_post_files_variable(lbuf, s+1, http_post_files, 0
TSRMLS_CC);
} else {
register_http_post_files_variable(lbuf, filename, http_post_files, 0
TSRMLS_CC);
}
--snip--
register_http_post_files_variable calls safe_php_register_variable:
--snip
if (override_protection || !is_protected_variable(var TSRMLS_CC)) {
php_register_variable_safe(var, strval, val_len, track_vars_array
TSRMLS_CC);
}
--snip--
override_protection is false, the only condition that checks whether the
variable name is accepted is the is_protected_variable call, passing the upload
name. The variable name is normalized using normalize_protected_variable() and
then checked for existence in the $_FILES array.
The normalization function normalize_protected_variable checks whether a
closing bracket is found, and otherwise uses the following string as index:
--snip--
indexend = strchr(index, ']');
indexend = indexend ? indexend + 1 : index + strlen(index);
--snip--
This implies that the index name can contain a opening bracket as well, which
will be accepted and passed directly to php_register_variable_safe.
The suggested patch adds a check to ensure that the leftover open brackets is
always zero. If not, it simply drops the upload (better safe than sorry).
Test script:
---------------
<?php
if (isset($_POST['submitted'])) {
if (isset($_FILES['test'])) {
if (isset($_FILES['test']['error'])) {
echo 'OK expected result';
} else if(isset($_FILES['test']['[error'])) {
echo 'Unexpected result';
}
} else {
echo 'OK expected result';
}
printf('<pre>%s</pre>', htmlspecialchars(print_r($_FILES, true)));
}
?>
<form enctype="multipart/form-data" method="post">
<input type="file" name="test[">
<input type="submit" name="submitted">
</form>
Expected result:
----------------
I expected to see "OK expected result" and an empty array dump because the name
is invalid.
Actual result:
--------------
The test script produces "Unexpected result".
The upload is accepted but the $_FILES array is corrupted:
Array
(
[test] => Array
(
[[name] =>
[[type] =>
[[tmp_name] =>
[[error] => 4
[[size] => 0
)
)
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=54374&edit=1
