Maksymilian Arciemowicz
Trust unworthy variables in PHP
by SecurityReason.Com
Maksymilian Arciemowicz
max [at] jestsuper [dot] pl
cxib [at] securityreason [dot] com
http://securityreason.com/key/Arciemowicz.Maksymilian.gpg
Recently, I have published a simple 'Full Path Disclosure and SQL Errors' bug,
which has presented lack of knowledge or secuirty of many programmists. All in
all, Full Path Disclosure is something not dangerous, but is also an error. The
majority of people tries to protect their PHP scripts, but they do not know
everything.
I had a dilemma with publishing the 'Full Path Disclosure and SQL Errors'
phpBB's bug - it is not harmful, but should not exist. We can easily find such
errors (phpMyAdmin, PostNuke, PHP-Nuke and many others). Even official PHP
website contains this bug.
Example:
http://php.net/?lang[]=BMS
- Result ---
Warning: setcookie() expects parameter 2 to be string, array given in
/home/local/Web/sites/www.php.net/include/site.inc on line 155
- Result ---
Let us see this code:
http://php.net/include/site.inc
--- Code ---
function mirror_setcookie($name, $content, $exptime)
{
if (!headers_sent()) {
if (is_official_mirror()) {
return setcookie($name, $content, time() + $exptime, '/',
'.php.net');
} else {
return setcookie($name, $content, time() + $exptime, '/');
}
} else {
return FALSE;
}
}
--- Code ---
By using setcookie() we are obligated to use it in accordance with
documentation:
http://pl2.php.net/manual/en/function.setcookie.php
bool setcookie ( string name [, string value [, int expire [, string path [,
string domain [, bool secure]]]]] )
So, we have to obey the rules. We have to use string (lang=PL) and nothing else
(for example: array lang[]=cx).
PHP checks what was in the majority of functions declared.
- setcookie() code ---
PHP_FUNCTION(setcookie)
{
char *name, *value = NULL, *path = NULL, *domain = NULL;
long expires = 0;
zend_bool secure = 0;
int name_len, value_len, path_len, domain_len;
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|slssb", &name,
&name_len, &value, &value_len, &expires, &path,
&path_len, &domain, &domain_len, &secure) == FAILURE) {
return;
}
...
- setcookie() code ---
As you can see there is no array(), so this function returns an error causing
Full Path Disclousure.
There are also many other functions, which structure is similar to this
(htmlspecialchars is probably one of most popular).
Many people would say that this script:
<? echo htmlspecialchars($_GET['x']); ?>
is secure. And they are in big mistake, because - let us see the documentation
and find:
string htmlspecialchars ( string string [, int quote_style [, string charset]] )
_GET['x'] could be a string but it does not have to. If it is an array it
causes Path Disclosure.
But there are many things which are not mentioned in PHP documentation. For
example: function parse_url(). Recently, I have seen its implementation in some
script and autor seemed not to make a mistake but he did. Let us see and
analyze how parse_url() works
http://pl2.php.net/manual/en/function.parse-url.php
array parse_url ( string url )
There must be a string for the input.
---
Return Values
On seriously malformed URLs, parse_url() may return FALSE and emit a E_WARNING.
Otherwise an associative array is returned, whose components may be (at least
one):
* scheme - e.g. http
* host
* port
* user
* pass
* path
* query - after the question mark ?
* fragment - after the hashmark
---
Let us say we have this script:
---
<?
$formatuj='';
if(is_string($_GET['url'])){
$formatuj=parse_url($_GET['url']);
}
?>
---
Somebody will say that it will not cause any problems. In fact, in
documentation there is no information that port cannot be higher than 5 digit
number. Let us check it:
?url=http://securityreason.com:000080/
- Result ---
Warning: parse_url(http://securityreason.com:000080/) [function.parse-url]:
Unable to parse url in /www/hig.php on line 4
- Result ---
Using @ before parse_url() would be the solution, but it is not a difficult to
hide our bugs, because in this case the variable $formatuj is empty and
_GET['url'] exists as string.
Disabling errors would be the global solution, but it will not change the fact
that script should be resistant to these attacks.
We can play with PHP functions in many ways. We can even try to find XSS etc,
but it requires to put quite a lot effort in this game.
The solution is to analyze all input variables and all checkings (for example:
is_string). Error displaying could be disabled, but I would rather suggest
correcting the code.
- Example Solusion for phpBB 2.0.20 ---
File: memberlist.php
-line 38-45-
if ( isset($HTTP_GET_VARS['mode']) || isset($HTTP_POST_VARS['mode']) )
{
$mode = ( isset($HTTP_POST_VARS['mode']) ) ?
htmlspecialchars($HTTP_POST_VARS['mode']) :
htmlspecialchars($HTTP_GET_VARS['mode']);
}
else
{
$mode = 'joined';
}
-line 38-45-
Replace to:
- fix -
if ( (isset($HTTP_GET_VARS['mode']) || isset($HTTP_POST_VARS['mode'])) &&
(is_string($HTTP_GET_VARS['mode']) || is_string($HTTP_POST_VARS['mode'])) )
{
$mode = ( isset($HTTP_POST_VARS['mode']) ) ?
htmlspecialchars($HTTP_POST_VARS['mode']) :
htmlspecialchars($HTTP_GET_VARS['mode']);
}
else
{
$mode = 'joined';
}
- fix -
- Example Solusion for phpBB 2.0.20 ---
SecurityReason.Com 2006.05.06