Maksymilian Arciemowicz

[cxib () securityreason com]

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