cleaning function, overcleaning?

[Ninjakreborn]

I have a function I have been using to clean variables for quite a while now.

<?php
function deepclean($varinfo) {
$varinfo = strip_tags($varinfo);
$varinfo = htmlspecialchars($varinfo, ENT_NOQUOTES);
$varinfo = htmlentities($varinfo);
$varinfo = mysql_real_escape_string($varinfo);
return $varinfo; // Added this line

}
?>

I am even currently starting to modify it in my spare time to accept arrays and do the same thing with arrays.  My new version even has another variable to say whether or not to use addslashes or not.  I have been rewriting the whole function on the side, so I can start using it after it's tested.  I thought this was a good function, however I addeded it in a security include into a page (the function), then I tried cleaning a bunch of variables.  The login structure I cleaned stopped working, I had to take them out.

 

The script was.

 

<?php
session_start();

if(isset($_POST['userid']))
$userid= $_POST['userid'];
else
$userid= $_GET['userid'];
if(isset($_POST['user']))
$user= $_POST['user'];
else
$user= $_GET['user'];
if(isset($_POST['pass']))
$pass= $_POST['pass'];
else
$pass=$_GET['pass'];
if(isset($_POST['redirect']))
$redirect= $_POST['redirect'];
else
$redirect= $_GET['redirect'];
if(isset($_POST['remember']))
$remember=$_POST['remember'];
else
$remember= $_GET['remember'];

include($_SERVER['game_config']);

	if ((!$user && !$userid) || !$pass) {
include("login-header.php");
	print "Please fill out all fields.";
include("login-footer.php");
	exit;
	}
		$pass=md5($pass);
	if(is_numeric($userid))
	$logresq=mysql_query("select `id`,`user`,`pass`, `daylogins`, `donator`, `choice` from players where id='$userid' and pass='$pass'");
	else
	$logresq=mysql_query("select `id`,`user`,`pass`, `daylogins`, `donator`, `choice` from players where user='$user' and pass='$pass'");
	$logres = mysql_num_rows($logresq);
	echo mysql_error();
	$logresa=mysql_fetch_array($logresq);
	if ($logres <= 0) {
include("login-header.php");
	print "Login failed. If you have not already, please signup. Otherwise, check your spelling and login again.<br><br><a href=loginbyid.php>Click here if you know your ID, but not your username.</a>";
include ("login-footer.php");
	exit;
	} else {
	$ctime=time();
	mysql_query("UPDATE players SET logins=logins+1, daylogins=daylogins+1, lastlogout = lpv, readtopics = '' WHERE id = $logresa[id]");
	if($remember == 1){ //Remember Me!!!
		setcookie("cw_uid",$logresa['id'],mktime(date("H"), date("i"), date("s"), date("m"), date("d"), date("Y")+1),"/",$_SERVER['game_cookie_domain']);
		setcookie("cw_pw",$pass,mktime(date("H"), date("i"), date("s"), date("m"), date("d"), date("Y")+1),"/",$_SERVER['game_cookie_domain']);
	}

	$userid=$logresa[id];
	session_register("userid");
	session_register("pass");
if(isset($redirect))
header("Location: $redirect");
elseif($logresa['choice'] == "")
header("Location: chooseside.php");
elseif($logresa['donator'] == 0 && $logresa['daylogins'] == 0)
header("Location: intro.php");
else
header("Location: intro.php");
}
	?>

So the actual point is, for some reason when I run (deepclean) on all the variables, the login goes to a blank white page (no errors) and just doesn't login.  When I stop cleaning the variables they work fine.

That is what I don't understand, is it something I didn't notice with my system, or is it something instead to do with the login script.

 

Thanks for any feedback, thanks.

[boo_lolly]

that's a nice little function in my opinion, i believe i will use it myself in some of my code... but wouldn't this be the same effect?

<?php
function clean($var){
$var = mysql_real_escape_string(htmlentities(htmlspecialchars(strip_tags($var))));
        return $var;
}
?>

[Jenk]

Yes, it is over-cleaning.

 

Use each function only when the circumstance requires you use it. If you are inserting values to a database, use mysql_real_escape_string() (or variant for other DB's,) and only then. Use htmlentities() when you wish to display HTML markup safely, and only then. Use strp_tags() to remove HTML markup, and only then. Etc.

 

Your snippet (2nd) is also failing to check if $_GET['var'] is set before attempting to use it, btw.