My Washing Machine

[drisate]

Hey guys i am trying to build an effective code that will secure all my vars one shot for my board.

this is what i have so fare

 

function safeEscapeString($string){ 
if (get_magic_quotes_gpc()) { 
return $string; 
} else { 
return mysql_real_escape_string($string); 
} 
} 

function cleanVar($string){ 
$string = trim($string); 
$string = safeEscapeString($string); 
$string = htmlentities($string); 
return $string; 
} 

if (isset($_POST)){
$empty = $POST_ = array();
foreach ($_POST as $varname => $varvalue) {
    if (empty($varvalue)) {
        $empty[$varname] = $varvalue;
    } else {
        $POST_[$varname] = cleanVar($varvalue);
        $POST_[$varname] = $varvalue;
        echo "POST var $varname = $POST_[$varname]<br>"; // DEBUG
    }
}
}

if (isset($_GET)){
$empty = $GET_ = array();
foreach ($_GET as $varname => $varvalue) {
    if (empty($varvalue)) {
        $empty[$varname] = $varvalue;
    } else {
        $GET_[$varname] = cleanVar($varvalue);
        $GET_[$varname] = $varvalue;
        echo "GET var $varname = $GET_[$varname]<br>"; // DEBUG
    }
}
}

if (isset($_COOKIE)){
$empty = $COOKIE_ = array();
foreach ($_COOKIE as $varname => $varvalue) {
    if (empty($varvalue)) {
        $empty[$varname] = $varvalue;
    } else {
        $COOKIE_[$varname] = cleanVar($varvalue);
        //$COOKIE_[$varname] = $varvalue;
        echo "COOKIE var $varname = $COOKIE_[$varname]<br>"; // DEBUG
    }
}
}

if (isset($_REQUEST)){
$empty = $REQUEST_ = array();
foreach ($_REQUEST as $varname => $varvalue) {
    if (empty($varvalue)) {
        $empty[$varname] = $varvalue;
    } else {
        $REQUEST_[$varname] = cleanVar($varvalue);
        //$REQUEST_[$varname] = $varvalue;
        echo "REQUEST var $varname = $REQUEST_[$varname]<br>"; // DEBUG
    }
}
}

 

Works well for POST GET and REQUEST but form some reason when it gets to COOKIE it breaks ...

 

POST var name = admin

POST var pass = demo

POST var from = target=forum

GET var target = dologin

COOKIE var logintheme = cpanel

COOKIE var cprelogin = no

COOKIE var cpsession = closed

COOKIE var PHPSESSID = 820232b77016038d436e6f9ee5154929

REQUEST var target = dologin

REQUEST var name = admin

REQUEST var pass = demo

REQUEST var from = target=forum

REQUEST var logintheme = cpanel

REQUEST var cprelogin = no

REQUEST var cpsession = closed

REQUEST var PHPSESSID = 820232b77016038d436e6f9ee5154929

 

http://versatilebb.com/demo

Username:  admin

Pass: demo

 

I can't log in and when i change

$COOKIE_[$varname] = cleanVar($varvalue);

to

$COOKIE_[$varname] = $varvalue;

 

it works number one ...

 

This is my login script

 

    $getuser=mysql_query("SELECT * FROM $dbprefix"."_user WHERE name='$name' AND pass='$md5pass'", $db); 
	if ($getuser)
	{
	    if (mysql_num_rows($getuser)>0)
			{
					//session_cache_expire(30);
					//session_start();
	        $loguser=mysql_fetch_array($getuser);
					if ($loguser['allow_cookie']=="true" && !isset($cookie) && !empty($name))
					{
    				    setcookie("cookie[name]",$name,time() + 86400*365);
    						setcookie("cookie[md5pass]",$md5pass,time() + 86400*365);
					}
					if (session_register("loguser"))
					{
					    $now=date("Y-m-d H:i:s");
							$update=mysql_query("UPDATE $dbprefix"."_user SET lastlogin='$now', last_IP='$REMOTE_ADDR' WHERE ID='$loguser[iD]'");
					}
					if (!isset($pass))$pass="";
					$loguser['origpass']=$pass;
					$clear_read_messg=mysql_query("DELETE FROM $dbprefix"."_mread WHERE user_ID='$loguser[iD]'", $db);
					if ($target=="dologin")
					{
					    if (!empty($from))
							{
							    //$from=ereg_replace("target=","otarget=",$from);
  			            print("<meta http-equiv=\"refresh\" content=\"0; URL=$PHP_SELF?$from$PS\">");
							}
							else
							{
  			            print("<meta http-equiv=\"refresh\" content=\"0; URL=$PHP_SELF?target=forum$PS\">");
							}
					}

	    }
			else
			{
			    include("admin/sendpass.inc.php");
				  $md5_2=md5($pass);
			}
	}
	else
	{
	    print("error: "); echo mysql_errno($db) . " - " . mysql_error($db);
	}

[Barand]

I'd use

<?php
function safeEscapeString($string){ 
   $string =  get_magic_quotes_gpc() ? stripslashes($string) : $string; 
   return mysql_real_escape_string($string);  
} 

That way, they all go through mysql_real_escape_string() without writing \ to the database

 

[drisate]

Thx i made the change but i get the same problem.I can't connect when i filtrer $COOKIE_ but all the rest seems to be ok.

[drisate]

This is frustrating hehe i can't release the code before i get it secured ...

If i can get this 100% working i am gona contact a moderator to stiky a topic with it because theres alot of questions abbout XSS protection. And this code is fast and global. You just have to run it at the bigining of every pages

[Barand]

please explain code

 

    } else {
        $POST_[$varname] = cleanVar($varvalue);
        $POST_[$varname] = $varvalue;
        echo "POST var $varname = $POST_[$varname]<br>"; // DEBUG
    }

 

Clean the data and store in $POST_[$varname]

overwrite it with the uncleansed data ???

 

Having cleansed any POST, GET, COOKIE data, why do REQUEST data, which is going to be any POST, GET or COOKIE data?

 

I won't be stickying it

[roopurt18]

First off, I notice things like this:

  $_POST[...] = ...
  $POST_[...] = ...

Is that intentional or are you being careless?  Because if it's not intentional it's certainly wrong.

 

Then you have this code, which is basically duplicated for $_POST, $_GET, $_COOKIE, etc.:

<?php
if (isset($_POST)){
  $empty = $POST_ = array();
  foreach ($_POST as $varname => $varvalue) {
    if (empty($varvalue)) {
      $empty[$varname] = $varvalue;
    } else {
    $POST_[$varname] = cleanVar($varvalue);
    $POST_[$varname] = $varvalue;
    echo "POST var $varname = $POST_[$varname]<br>"; // DEBUG
    }
  }
}
?>

To start with, $_POST is always set so your if() statement is useless as it's always true, at least AFAIK.  Further, it's good web practice when your users enter invalid data into forms to repopulate the form with the data they entered.  But here you are applying htmlentities() and mysql_real_escape_string() to the user's data. 

 

So what happens when your user enters this:

  In PHP, < is used for less than and > is used for greater than.

Let's say your site invalidates the form because the user didn't enter their e-mail address in another field.  Your site redisplays the form and populates it with:

  In PHP, < is used for less than and > is used for greater than.

The user doesn't notice their message has changed; when they enter their e-mail address and submit the form, your site will change it again to:

  In PHP, < is used for less than and > is used for greater than.

 

Later on, someone else views this post (or whatever it is) and sees:

  In PHP, < is used for less than and > is used for greather than.

IMO, not so good!

 

Here is a recursive function that is a one-size fits all:

<?php
  /**
   * Multi-use function for cleaning user data.  Can be used to clean
   * entire arrays or a single value.
   * @param array|scalar Data to clean
   * @param [bool] true to trim
   * @param [bool] true to mysql_real_escape_string
   */
  function my_super_clean($data, $trim = true, $mysql_escape = true){
    // We don't want to call get_magic_quotes_gpc() more than once
    // so we use a static variable
    static $gpc_on = null;
    $gpc_on = $gpc_on === null ? get_magic_quotes_gpc() : $gpc_on;

    // $data can be an array or a single value
    if(is_array($data)){
      foreach($data as $k => $v){
        $data[$k] = my_super_clean($v);
      }
    }else{
      // Do we stripslashes?
      $data = $gpc_on ? stripslashes($data) : $data;
      // Do we trim?
      $data = $trim ? trim($data) : $data;
      // Do we prepare for database
      if($mysql_escape){
        // If the value is numeric we do not have to escape it
// or enclose it in single quotes.
// If the value is not numeric we must enclose in single
// quotes and escape
$data = is_numeric($data) 
      ? $data 
      : "'" . mysql_real_escape_string($data) . "'";
      }

    }
    return $data;
  }
?>

Notice how I've completely left off any calls to htmlentities()?  In general, you do not want to call htmlentities() on data as it's inserted in the database.  For example, pretend I entered << into a field that is stored in a VARCHAR(2) column.  If you call htmlentities(), your code is going to try and insert << into a column that can only fit two chars.  Wrong.

 

The only thing you store in the database is mysql_real_escape_string()'ed data, optionally trim()'ed.  You call htmlentities() after you retrieve the data, just before it is displayed.  Also, since you only need to call htmlentities() when displaying to a html-compatible viewer, why bother calling it as you're inserting the data.  After all, who says you're always going to display back to a web browser?  You can't predict the future functionalities of your application so don't store your data geared for use in a single purpose.

 

Now, the function I've given you above can easily be accompanied by another clean function that will clean $_POST, $_GET, and $_COOKIE, etc. at the beginning of your script:

  function super_clean_auto_globals(){
    $g = Array( '_POST', '_GET', '_COOKIE' );
    foreach($g as $v){
      // trim() but not mysql_real_escape_string()
      ${$v} = my_super_clean(${$v}, true, false);
    }
  }

 

Now hopefully your site uses mod_rewrite to route all requests through a single index.php file; if so, at the top of it you can place:

  super_clean_auto_globals();

Which will clean up all of your auto globals for use throughout the entire site.

 

Now, let's say you are inserting data into the database; the recursive function comes in handy again:

  function insert_db($val1, $val2, $val3){
    $Clean = Array();
    $Clean['col1'] = my_super_clean($val1);
    $Clean['col2'] = my_super_clean($val2);
    $Clean['col3'] = my_super_clean($val3);
    $sql = "
      INSERT INTO `table` (`col1`, `col2`, `col3`) VALUES (
        {$Clean['col1']}, {$Clean['col2']}, {$Clean['col3']}
      )
    ";
    $q = mysql_query($sql);
    // finish up the function
  }

 

Now this thread might be worthy of a sticky!

 

[drisate]

omg thanks! This is perfect from a to z and yes this would be great for a sticky version. I am gona PM a few moderators untile one does lol

thanks a lot. VersatileBB should be fine using that.

 

But i do have one question

 

abbout

 

  function insert_db($val1, $val2, $val3){

    $Clean = Array();

    $Clean['col1'] = my_super_clean($val1);

    $Clean['col2'] = my_super_clean($val2);

    $Clean['col3'] = my_super_clean($val3);

    $sql = "

      INSERT INTO `table` (`col1`, `col2`, `col3`) VALUES (

        {$Clean['col1']}, {$Clean['col2']}, {$Clean['col3']}

      )

    ";

    $q = mysql_query($sql);

    // finish up the function

  }

 

Why would you use that if everything is already filtred by using super_clean_auto_globals();?

[Daniel0]

I always secure against SQL injections by using prepared statements using PDO and I secure against XSS by filtering the data just before it's output. Also, why trim() it? It'll just remove whitespace from the start and end.

[roopurt18]

Why would you use that if everything is already filtred by using super_clean_auto_globals();?

Because the super_clean_auto_globals() call didn't escape things for the database.  Basically, stripslashes() is applied to the auto globals if Magic Quotes is on at the beginning of the script, but the data isn't safe to place into the database.  Therefore, just before inserting into the database you must clean it again.  I guess I should point out there is a slim chance by the time you are cleaning for database insertion, there is a chance the data cleaned will contain \\ or \', in which case the clean function will revert it to \ or ' respectively.  You could get around this with a 4th optional URL parameter, called $skip_stripslashes or something and initializing it to false.

 

Also, my use of a variable named $Clean is a personal preference.  The only values I insert int my SQL statements are those that come out of a $Clean array.  I do this intentionally so I force myself to clean everything and I usually do it very close to where it's used.  If you clean a variable on line 10 and insert it on line 80, how do you know it didn't change to something else in between?  Also, you have to admit my structure for SQL statements is very easy to read and maintain.

[drisate]

I applyed the change but for somereason it's not fully working

http://versatilebb.com/demo/dereferrer.php?url=>"><ScRiPt%20%0a%0d>alert(380609953)%3B</ScRiPt>

 

i used this at the bigining of my central page

 

  /**
  * Multi-use function for cleaning user data.  Can be used to clean
  * entire arrays or a single value.
  * @param array|scalar Data to clean
  * @param [bool] true to trim
  * @param [bool] true to mysql_real_escape_string
  */
 function my_super_clean($data, $trim = true, $mysql_escape = true){
   // We don't want to call get_magic_quotes_gpc() more than once
   // so we use a static variable
   static $gpc_on = null;
   $gpc_on = $gpc_on === null ? get_magic_quotes_gpc() : $gpc_on;

   // $data can be an array or a single value
   if(is_array($data)){
     foreach($data as $k => $v){
       $data[$k] = my_super_clean($v);
     }
   }else{
     // Do we stripslashes?
     $data = $gpc_on ? stripslashes($data) : $data;
     // Do we trim?
     $data = $trim ? trim($data) : $data;
     // Do we prepare for database
     if($mysql_escape){
       // If the value is numeric we do not have to escape it
// or enclose it in single quotes.
// If the value is not numeric we must enclose in single
// quotes and escape
$data = is_numeric($data) 
      ? $data 
      : "'" . mysql_real_escape_string($data) . "'";
     }

   }
   return $data;
 }

 function super_clean_auto_globals(){
   $g = Array( '_POST', '_GET', '_COOKIE', '_REQUEST' );
   foreach($g as $v){
     // trim() but not mysql_real_escape_string()
     ${$v} = my_super_clean(${$v}, true, false);
   }
 }

super_clean_auto_globals();

[roopurt18]

I always secure against SQL injections by using prepared statements using PDO and I secure against XSS by filtering the data just before it's output. Also, why trim() it? It'll just remove whitespace from the start and end.

I don't use PDO in my applications, so I came up with an alternative style.

 

Ditto on your note about XSS.

 

As far as trim() goes, that's a personal preference of mine.  There is very little to be gained, IMO, by allowing users to surround their data with whitespace.  Despite my comments about htmlentities(), 99% of all data in a MySQL database is going to reappear in a web browser, which just ignores extra white space.  Or, if something like nl2br() is used, it just increases the opportunity for users to abuse your system.

[roopurt18]

I applyed the change but for somereason it's not fully working

http://versatilebb.com/demo/dereferrer.php?url=>"><ScRiPt%20%0a%0d>alert(380609953)%3B</ScRiPt>

 

i used this at the bigining of my central page

Can you post the code for dereferrer.php?

[drisate]

Yeah

 

if (isset($_GET)) {
$url=$_GET['url'];
} else {
    $url=$HTP_GET_VARS['url'];
}

$url=urldecode($url);
$url=urldecode($url);
$url=stripslashes($url);

//$url=eregi_replace("\/\/\/","",$url);
$url=eregi_replace("\&quot\;","\"",$url);
$url=eregi_replace("^([\"\']*)(.*)([\"\']*)$","\\2",$url);
$url=eregi_replace("(.*)\\\'$","\\1",$url);
if (ereg("://", $url)) 
{
    echo "<meta http-equiv=\"refresh\" content=\"0; URL=" . $url ."\">";
}
else 
{
    echo "<meta http-equiv=\"refresh\" content=\"0; URL=http://". $url . "\">";
}

[Daniel0]

As far as trim() goes, that's a personal preference of mine.  There is very little to be gained, IMO, by allowing users to surround their data with whitespace.  Despite my comments about htmlentities(), 99% of all data in a MySQL database is going to reappear in a web browser, which just ignores extra white space.  Or, if something like nl2br() is used, it just increases the opportunity for users to abuse your system.

 

I see. That makes sense.

[roopurt18]

I'm wondering why you're doing this with meta tags.  Anyways, my comments about htmlentities() still stand.  You call htmlentities() on user supplied data just before displaying it:

$url = htmlentities($url);  // <-- INSERT THIS LINE
if (ereg("://", $url)) 
{
    echo "<meta http-equiv=\"refresh\" content=\"0; URL=" . $url ."\">";
}
else 
{
    echo "<meta http-equiv=\"refresh\" content=\"0; URL=http://". $url . "\">";
}

 

But I have to ask, why not just do this:

// Note this is the entire file
$url = isset($_GET) && isset($_GET['url']) ? $_GET['url'] : $HTTP_GET_VARS['url']
if(strlen($url)){
  header('Location: ' . $url);
  exit();
}else{
  echo 'No URL provided!';
}

 

Also, your server must be using $_GET because if it wasn't you'd notice you've misspelled $HTTP_GET_VARS.

[drisate]

Yeah much better lol I would greatly appreciate one or 2 like you on my dev team for versatilbb lol

 

I get this message

Parse error: syntax error, unexpected T_IF in /home/versatil/public_html/demo/dereferrer.php on line 18

 

17 -> $url = isset($_GET) && isset($_GET['url']) ? $_GET['url'] : $HTTP_GET_VARS['url']

18 -> if(strlen($url)){

19 -> header('Location: ' . $url);

 

never mind just saw you where missing a ;

 

Now i get this

Warning: Header may not contain more than a single header, new line detected. in /home/versatil/public_html/demo/dereferrer.php on line 19

[Barand]

missing ; at end of line

[roopurt18]

Put this at the top of your file:

<?php
// Note this is the entire file
$url = isset($_GET) && isset($_GET['url']) ? $_GET['url'] : $HTTP_GET_VARS['url'];
if(strlen($url)){
  header('Location: ' . $url);
  exit();
}else{
  echo 'No URL provided!';
}
?>

 

And if it works correctly just delete the rest of the file.

[drisate]

It works but if you type

http://versatilebb.com/demo/dereferrer.php?url=>"><ScRiPt%20%0a%0d>alert(380609953)%3B</ScRiPt>

 

you get

 

Warning: Header may not contain more than a single header, new line detected. in /home/versatil/public_html/demo/dereferrer.php on line 20

 

-> 19 if(strlen($url)){

-> 20 header('Location: ' . $url);

-> 21 exit();

[drisate]

weird and this

http://versatilebb.com/demo/dereferrer.php?url=http://versatilebb.com/demo/dereferrer.php

 

returns

No URL provided!

 

lol

[Barand]

It would.

 

It passes dereferrer.php as the url

 

So it gets redirected to dereferrer.php

 

But this time there is no url, so you get "No URL provided"

[drisate]

Oh that makes sens lol thx for the help guys i think i can do something with everything submited here. And i still thinks your code should be stikyed somw how. I just relized your a moderator lol why not doing it hehe