Maksymilian Arciemowicz

[cxib@xxxxxxxxxxxxxxxxxx]

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