Simple Function To Check Any and All User Input - HTML Forums

Paul · 02-23-2007, 02:57 PM

You should add the following function to your script to elimnate the possibility of people passing binary data through your forms which is usually used by hackers to gain access to your entire web site and possibly the entire server you are on.

PHP Code:


			
<?

function secure($var) {

    $var = strip_tags($var);

    $var = trim(htmlentities($var, ENT_QUOTES));

    return $var;

} 

?>

usage

PHP Code:


			
$form_variable = secure($_POST['form_variable']);

You should apply this function to every variable that a user passes to your script using a form.

Thanks to another member in another forum really giving me crap for not doing this in a script.

dimeric · 02-25-2007, 07:21 PM

You don't need to have the htmlentities in there. Especially if you want to allow html input. htmlentities should be applied to unsafe data before displaying it to users, it shouldnt be applied to input.

Paul · 02-25-2007, 10:34 PM

Quote:

Originally Posted by dimeric

You don't need to have the htmlentities in there. Especially if you want to allow html input. htmlentities should be applied to unsafe data before displaying it to users, it shouldnt be applied to input.

Thanks for your input, if I don't need that in there I'll go a head and take it out in my script.

But isnt that the only real way to protect your script from binary input in php 4.x.x ? I know strip_tags does it but I think that only works properly in PHP 5, no?

erisco · 02-26-2007, 11:19 AM

A lot of functions are binary safe... this is an interesting security issue that I have not looked a lot into. I want to do some research tonight... that function looks fine but I wonder if there isn't already something else (built-in) to safe-guard scripts.. you'd think so given that this is a known security problem.

Paul · 02-26-2007, 12:04 PM

If you find anything please let me know. I've been attacked using this method on several different occasions, usually when dealing with email/contact forms.

erisco · 02-27-2007, 04:48 PM

Well I have failed to find anything on "binary attacks". If such a thing did exist, then I still fail to see why safe-guarding against HTML injection has anything to do with it.

Binary-safe on functions means they can read binary, as I have learned. However, be it binary or not it doesn't matter.. the security issues are caused by not validating user input to what it needs to be. Htmlentities converts items found in the html table (exact name I am not sure of.. functions to work with it though), trim knocks off whitespace before and after a string.. and strip_tags removes all html tags from a string. Again, I do not see how this translates into anything but avoiding HTML injection. This is absolutely no threat to PHP or your server.. just to your site on the client-end..

That said, although perhaps wrong, a custom function may not be needed. After all, if we do not want to have html we simply use strip_tags.. and if we don't want to be stripping out intentional brackets then htmlspecialchars is just as viable. If anything, this is a good convenience function.

PHP Code:


			
<?php
funciton h($string) {
    return htmlspecialchars($string);
}
?>

Borrowed from the cakePHP framework.

Paul · 02-28-2007, 12:24 PM

I will need to look at this a little more but here is a good article about hex injection:

http://www.securephpwiki.com/index.php/Email_Injection

erisco · 02-28-2007, 03:40 PM

Header injection is not necessarily a security issue, but rather allows someone to change where your emails are going and/or say where they came from. It certainly shouldn't be a risk of PHP or server security... there isn't any way you should be giving out any credentials over form email is there?

The hex injection was to insert a line feed (newline "\n") which again resulted in header injection. This time it was used to abuse Bcc and Cc (and even multiple to addresses). Stripping html, removing whitespace, it does not prevent this kind of trouble whatsoever.

What will protect this kind of injection is taking appropriate caution.. which as the article has stated many projects have already done. At the bottom it lists different packages you could safely use to avoid this type of email nuisance. It also does a little explanation of how you could protect your emails for yourself. But, unfortunately Paul I do not believe the function you have provided will work against these exploits.

dimeric · 03-04-2007, 03:14 PM

I think your (Paul not ericso) confusing injection attacks (which are attacking either the browser or datastore) with attacks against php, such things like buffer overflow etc are quite different and arn't really defended against by parsing user input. Instead you just ensure that your code has sufficient exception handling and doesn't call unsafe functions (i'm not so good when it comes to php stuff as far as security goes) you should of course also parse user input, but this is just a first line of defense (it is pretty much the only line of defence for injection).

Also while php function may read binary data, the input is handled as a string, and you can force this with typecasting. Which would avoid any chance of it being handled as binary data (unfotunately this is a problem with loose typing, the solution is of course to use a strongly typed language!).

Either way do update if you find anything else about these binary attacks

Paul · 03-04-2007, 03:54 PM

Thanks to both of you for all the input. I've been hacked too many damn times so at this point I dont want to run anything that could be exploited in anyway.

I added this function to my code today and deleted the htmlentities part. What's strange is that with this function if you check it with isset() it always returns true

. For example.

I changed it to the following and still same problem:

PHP Code:


			
#Check User Input

function secure($var) {

    if($var) {

        $var = strip_tags($var); 

        $var = trim($var); 

        return $var;

    }

    else {

        return false;

    }

}

So if I run the following I //code here always gets executed even if $submitted is not set:

PHP Code:


			
$submitted = secure($_POST["submit"]);

if(isset($submitted) {

   //code here

}

yet when I do this it works fine:

PHP Code:


			
if($submitted) {

    //code here

}

Strange, no?

dimeric · 03-04-2007, 06:21 PM

Um of course it always evaluates true.

PHP Code:


			
$submitted = secure($_POST["submit"]); 

if(isset($submitted) { 

   //code here 

}

This either sets $submetted to the value off $_POST["submit"] or it sets it the boolean value false (a nice example of the weak typing). isset() then checks if this variable has a value associated with it (There would be a subtlety here if your variable is a reference type but thats slightly beyond this).

$submitted always has a value once secure() has been run, so isset() always returns true.

You could change it like this:

PHP Code:


			
#Check User Input 

function secure($var, $outVar) 

{ 

    if(isset($var)) 

    { 

        $var = strip_tags($var); 

        $var = trim($var); 

        $outVar = $var;

        return true; 

    } 

    else 

    {

        return false; 

    } 

} 





//then use it like so

$submitted = "";

if(secure($_POST["submit"], $submitted))

{ 

   //code here 

   //You can now use $submitted

}

or you could do this

PHP Code:


			
#Check User Input 

function secure($var) 

{ 

    if(isset($var)) 

    { 

        $var = strip_tags($var); 

        $var = trim($var); 

        return $var;

    } 

    else 

    {

        return;  //here this effectively returns void

    } 

}

But its better practice to have functions return only 1 type but with different values (as the first example does).

erisco · 03-04-2007, 10:13 PM

or why not...

PHP Code:


			
function secure($string) {

    if(!empty($string)) {

        return trim(strip_tags($string));

    }

    return false;

}



$submitted = secure($_POST['unsafe']);



if ($submitted !== false) {

    echo $submitted;

}

I think that is better yet.

dimeric, I do not see how that is "better practice" to only return one type. IMHO, it depends on what the function is. I'd say quite a few built-in functions have multiple return values. Returning false on function failure is not that bad of an idea.

Paul · 03-04-2007, 10:55 PM

I was under the impression that if you return false it becomes null and in return isset would return false. Thanks both for the help.

dimeric · 03-05-2007, 06:43 AM

The reason i say better practice is because it provides a feedback as to whether a function has worked or not, regardless of input. Also its good practice as thats how most other languages work. Look at professional API's and you will find they like single return types (for all sort of reasons, like exposing to webservices etc) but its just nicer. Also PHPs type comparisons are notoriously "iffy"!

Either way glad you have it sorted, you just have to remeber that false is a value.

erisco · 03-05-2007, 11:24 AM

dimeric, fair argument. I propose this:

PHP Code:


			
function secure($string) {

    return trim(strip_tags($string));

}



if (!empty($_POST['unsafe'])) {

    $submitted = secure($_POST['unsafe']);

}

null, true, false, are all three entirely different values. However, take this example:

PHP Code:


			
$foo = null;

if (isset($foo)) echo 'foo exists';

The variable is equal to null, the variable exists, therefore isset() indeed proves true.