PHP sprintf considered harmful?

问题:

A few years ago, I learned about Format String Attacks in C the hard way. Now, I recently saw some PHP code like this:

<?php
echo sprintf($_GET['format'], $_GET['value1'], $_GET['value2']);

I tried run this like this with $_GET['format'] set to strings like %s%s%s..., but PHP just exists with PHP Warning: sprintf(): Too few arguments in file.php on line 2. Isn't it still possible to do a format string attack?

回答1:

Not in any traditional sense, as PHP's sprintf doesn't support any of the really dangerous conversions like %n. A user-controlled format string can still cause some limited havoc (consider %99999999s), but about the worst I think it could do would be to consume memory and time.

回答2:

I also found an integer overflow. Which leads to this:

<?php
echo sprintf('%2147483646$s', "foo"); # Warning: Too few arguments
echo sprintf('%2147483647$s', "foo"); # Warning: Argument number must be greater than zero

I filed this as PHP Bug #61531. I am not sure, if it may be exploitable.

回答3:

Higher-level languages tend to validate the number of arguments passed instead of blindly trusting the programmer. You will need to find another attack vector.

回答4:

Not sure if there is a PHP sprintf() vulnerability, but, example code appears to be a prime example of how not to do things.

As far as I'm concerned, sprintf should help you lock down a string, not the other way around. For example, this is what I'd consider useful reason to use sprintf():

$sql = sprintf(
    "insert into table (myname, myvalue) values ('%s', '%s')",
    mysql_real_escape_string( $_GET['name'] ),
    intval( $_GET['value'] )
);

A more useful reason to use sprintf is to borrow its cousin, vsprintf and reuse constantly:

// takes variable number of parameters
function sanitize() {
    $args = func_get_args();
    $sql = array_shift( $args );
    foreach( $args as $k => $v ) $args[$k] = mysql_escape_string( $v );
    $safe_sql = vsprintf( $sql, $args );
    return( $safe_sql );
}

With it, you can safely isolate parameters from queries and assume they will be sanitized:

$t = mysql_query(sanitize(
    "insert into mytable (myname, myvalue) values ('%s', '%s')",
    $_GET['name'],
    $_GET['value']
));