Security concern using while(list($key,$value)=each($_REQUEST))

[lalomarquez]

Dear friends,

 

I have multiple strings sent by mixed post and get. Some are numbers for get "includes" to get existing pages from a directory (1.php, 2.php, 3.php, etc.) and others are to do text queries to mysql (I'm using mysql_real_escape_string() to clean them up a little bit more). In my .htaccess file I have the following:

 

RewriteEngine on

RewriteCond %{QUERY_STRING} http[:%] [NC]
RewriteRule .* /------------http----------- [F,NC]
RewriteRule http: /---------http----------- [F,NC]

 

and in the main index file, which is the default that calls every other file, I have this at the top:

 

if(preg_match("/http:/i", urldecode(getenv("REQUEST_URI").getenv("QUERY_STRING")))) {
Header( "HTTP/1.1 503 Service Unavailable" );
exit;
}

 

Last night I learn that I could use code below to get all variables sent instead of writing them down one by one as I always did before. But I'm not sure if this method is open to attacks.

 

I'm not sure if the server of the friend I doing the script for has globals on or off and I'm not really sure if I would find out soon enough, so I want to cover for both possibilities.

 

So, I need that the numeric variables that I use (like $pid, for example) are in fact integers, and the text variables are getting sanitized before processing.

 

In my current test the code is working fine, but I'm not sure if it is prone to hacking. I sincerely thank you in advance if you could take a look at it and give me some pointers.

 

The code:

 

while ( list ( $key, $value ) = each ( $_REQUEST ) ) {

    if ( (int) $value == 0 ) {

        $value = trim ( strip_tags ( urldecode ( $value ) ) );  

        if ( get_magic_quotes_gpc () ) {

            $value = trim ( stripslashes ( strip_tags ( urldecode ( $value ) ) ) );

        } else {

            $value = trim ( strip_tags ( urldecode ( $value ) ) );

        }


    } else {

        $value = ( int ) $value;

    }

}

[ucffool]

You can use sprintf() or regular expressions to clean up stuff as well, the rule of thumb is if you have user input, DO NOT trust it. anyone can POST or GET to one of your pages, scrub it for what you expect. Always specify what you EXPECT, not what to exclude (a losing battle).

[lalomarquez]

Thank you for replying! I just learned about sprintf() no too long ago, I've been using it although I don't really understand how it works. I just copy and paste from one script to another. This is a very short example:

 

                $query = sprintf ("UPDATE status
                                   SET description='%s'
                                   WHERE id='$id'
                                   LIMIT 1",
                                  mysql_real_escape_string($description, $link)
                                  );

 

As I said: I use it liberally but I don't understand it. Although as long as it puts another layer of security on top, fine with me ;-)

[ucffool]

I had that feeling too until I had to write a book about it (in a way someone else wouldn't take an hour to figure out how it works), so don't feel bad. Check my sig, there is a free PDF version of my book and you can check out the sprintf() function explanation (takes 3 pages) if you want.

If not, that's fine too.

[Cep]

The link in your sig is broken two http:// 's

[ucffool]

It's amazing what a fat finger will do sometimes

Fixed, thanks.

[lalomarquez]

I had that feeling too until I had to write a book about it (in a way someone else wouldn't take an hour to figure out how it works), so don't feel bad. Check my sig, there is a free PDF version of my book and you can check out the sprintf() function explanation (takes 3 pages) if you want.

If not, that's fine too.

 

Hey!! At last I get to know someone famous!!  Thank you very much, I'll take a look at the sprintf (on page 47 ;-)) I have read only 1 1/2 books on php: a very basic one with which I got interested, and the one I'm trying to finish these days: AJAX and PHP: Building Responsive Web Applications. What I would like to read is a book that deals with making robust and secure sites and well structured shortcuts for coding.

 

At the end, I end up with my code like this:

 

while(list($key, $value) = each($_REQUEST)) {

    if ( ctype_digit ( $value ) == TRUE ) {

        $value = (int) $value ;

    } else {

        if (get_magic_quotes_gpc()) {

            $value = trim(stripslashes(strip_tags(urldecode($value))));

        } else {

            $value = trim(strip_tags(urldecode($value)));

        }

    }

}

if ( isset ($pid) AND file_exists ( "includes/$pid.php" ) ) {

    include_once ( "includes/$pid.php" );

} else {

    $url = $_SERVER["REQUEST_URI"];
    header ( "HTTP/1.0 404 Not Found" );
    header ( "location: index.php?pid=0&fn=404&op=$url" );
    exit;

}

 

Thanks again for your help and for sharing. Best regards.

[lalomarquez]

O-oh... Don't mind my code, is all wrong. It doesn't do what I thought it would do. I had register_globals on in my server, that's why I thought it was working, but after turning it off, I realize that what I want is much more complicated of what I thought.