Message-ID: <20060516203711.29950.qmail@securityfocus.com>
Date: 16 May 2006 20:37:11 -0000
From: cxib@...urityreason.com
To: bugtraq@...urityfocus.com
Subject: 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

Powered by blists - more mailing lists