Page 1 of 1
Input Sanitization
Posted: Mon Nov 21, 2011 12:40 am
by Hermit TL
Hello everyone! This place looks great. I'm not new to programming, but I am new to php.
I'm trying to write a function to sanitize a users' input before it's passed to a variable.
The only things users will be allowed to input is plain text and URI's.
I was hoping that someone could
examine the function I have written and provide some constructive criticism or pointers with the primary focus being on security. Thanks in advanced.
Code: Select all
<?php
//****************************************
//****************************************
//** security.php by Hermit TL
//**
//** Version 0.1 Alpha
//**
//** Supports plain (english) text
//** and URL input only.
//**
//** Returns NULL on failure.
//****************************************
//****************************************
function security_filter ($input, $what) {
switch ($what){
case "string":
switch (is_string($input)){
case true:
$input = filter_var($input, FILTER_SANITIZE_STRING, FILTER_NULL_ON_FAILURE);
break;
default:
$input = NULL;
break;
}
break;
case "url":
$chk = filter_var($input, FILTER_SANITIZE_URL, FILTER_NULL_ON_FAILURE);
switch($chk){
case !NULL:
$input = filter_var($input, FILTER_SANITIZE_URL, FILTER_NULL_ON_FAILURE);
break;
default:
$input = NULL;
break;
}
break;
default:
$input = NULL;
}
return $input;
}
?>
Re: Input Sanitization
Posted: Mon Nov 21, 2011 3:15 am
by mikeashfield
I'd do it something like this myself:
Code: Select all
<?php
if (isset($_POST['input_value'])) {
$cleanse_value=$_POST['input_value']
if (preg_match('((mailto\:|(news|(ht|f)tp(s?))\://){1}\S+)', $cleanse_value)) {
$cleansed_value=$cleanse_value;
// Some code here to handle a valid URL.
}
elseif (preg_match('^\s*[a-zA-Z,\s]+\s*$^', $cleanse_value)) {
$cleansed_value=$cleanse_value;
// Some code here to handle plain text.
}
else {
// Some code here to handle invalid input.
}
}
?>
Re: Input Sanitization
Posted: Mon Nov 21, 2011 3:30 am
by Hermit TL
Why?
What would be the difference/advantage to using pattern matching as oppose to the FILTERs provided by PHP? I'm not oppose to writing functions that have equivalents already available due the fact that you are more aware of and have more control over what is actually happening but aren't the FILTERs written specifically for doing what your pattering matching code is illustrating?
Re: Input Sanitization
Posted: Mon Nov 21, 2011 3:31 am
by Mordred
Unless you post how you intend to use it, there's little we can tell from this. It's good for some things and bad for other things. Depends on context, which we lack.
Also, the switch with two cases is more commonly known as if

The most dangerous thing I can see right now is that you're passing "invalid" values right through, instead of returning NULL on them.
@
mikeashfield: your code is not equivalent (and buggy), so it's not a viable replacement.
Re: Input Sanitization
Posted: Mon Nov 21, 2011 3:48 am
by Hermit TL
@Mordred, Sorry about that I thought I provided enough information(or I am not understanding what I left out).
Mainly it's used for filtering the users login input, as such:
$username = security_filter($_POST['username'], "string");
in an effort to prevent invalid input before actually using the users input to query the mysql database.
(and yes, I know about the if statement, but with it formatted this way I find it would be easier (for me anyways) to add another option in the future if I so desire.)
Can you please explain to me how invalid values are going right through? As far as I can tell, it returns a filtered value or null; I don't see where (or how) the invalid value is being returned by the function.
Re: Input Sanitization
Posted: Mon Nov 21, 2011 4:55 am
by Mordred
If you want to put data in the database, run it through mysql_real_escape_string (or the equivalent function in your db driver). Nothing else is equivalent. FILTER_SANITIZE_STRING is particularly not sufficient
My mistake about the invalid values, you're right. Your function will sometimes return false instead of NULL on filter failure though (FILTER_NULL_ON_FAILURE is only for FILTER_VALIDATE_BOOLEAN, at least according to the docs)
Re: Input Sanitization
Posted: Mon Nov 21, 2011 5:26 am
by Hermit TL
Thank you, Mordred. You're right about the FILTER_NULL_ON_FAILURE flag, it may not be doing anything (according to the docs) but it doesn't seem to be hurting the script as far as I can tell, so I'll leave it for now until I run a few tests.
It seems that I missed a most important step by not using mysql_real_escape_string as SQL injection was my primary concern and the whole reason I wrote the function.
I've modified my function as per your recommendation as such:
Code: Select all
<?php
//****************************************
//****************************************
//** security.php by Hermit TL
//**
//** Version 0.2 Alpha
//**
//** Supports plain (english) text
//** and URL input only.
//**
//** Returns NULL on failure.
//****************************************
//****************************************
function security_filter ($input, $what) {
$input = mysql_real_escape_string($input);
switch ($what){
case "string":
switch (is_string($input)){
case true:
$input = filter_var($input, FILTER_SANITIZE_STRING, FILTER_NULL_ON_FAILURE);
break;
default:
$input = NULL;
break;
}
break;
case "url":
$chk = filter_var($input, FILTER_SANITIZE_URL, FILTER_NULL_ON_FAILURE);
switch($chk){
case !NULL:
$input = filter_var($input, FILTER_SANITIZE_URL, FILTER_NULL_ON_FAILURE);
break;
default:
$input = NULL;
break;
}
break;
default:
$input = NULL;
}
if ($input == false){
$input = NULL;
}
return $input;
}
?>
Re: Input Sanitization
Posted: Mon Nov 21, 2011 7:45 am
by Mordred
No.
You can't have a function that magically cleans up everything and makes all data safe for everything. Can't be done, don't try.
Instead, every time before inserting data into a mysql query you should run mysql_real_escape_string().
If I were you, I would scrap that function because it's too easy to use it incorrectly.
Re: Input Sanitization
Posted: Mon Nov 21, 2011 8:25 am
by Hermit TL
I never once said that this function was for 'magically making all data safe for everything'. I said this is to filter a users' input before using it to query the database (a query does not necessarily mean inputing data into the database). Furthermore, I do not see how simply using mysql_real_escape_string() on every single variable that is passed to a query is any different than passing the variable through a function which includes the same command; please elaborate. In addition (correct me if I'm wrong), isn't the point of a function to avoid having to rewrite code? I also fail to see how this function can be 'used incorrectly'; I wrote it, if it is being used incorrectly, it's my own fault, not the users (and what the user is doing is what I am concerned with). And regarding your statement "Can't be done, don't try." What kind of attitude is that? Of course it can be done, maybe not the way you're thinking, maybe not easily, maybe not even worth the effort, and probably won't be portable, and yes, maybe I'll fail and waste a bunch of time, but not to try is NOT an option.
You seemed rather helpful examining my function and willing to help me improve it but when I added that line of code to run the input variable through mysql_real_escape_string() you suddenly take the stance of 'don't make a filter function', what's the deal?
Like I said; I'm new to php. If your going to shoot me down, I would prefer you provide the reasoning behind why my function is flawed rather than providing your advice in the form of what appears to be an opinion.
That said, as far as I can tell, this function does exactly what it was programmed to do. If you can show me a type of input (or whatever) that can make it through the function intact enough to result in undesirable mysql queries; I would love to see it so that I may improve my understanding of php.
Re: Input Sanitization
Posted: Mon Nov 21, 2011 10:20 am
by Mordred
Sorry, it's hard to provide emotion cues over the Internet. It was a quick reply, my intention was brevity, not shooting you down

Please accept my apologies.
I'll elaborate a bit (although I still don't have much time

)
Every "dangerous" function (like mysql_query with the danger of SQL injection, or echo with the danger of XSS, etc.) has its own mechanism for making input safe to be used.
You can't write a function that works for all dangerous functions. If you run some data through all safety functions they are likely to cancel each other in strange and unforeseen ways.
If you want, you can strip tags from text before using it in a query (although it is wrong to generally do so) but whatever you do to a string, mysql_real_escape_string should be the *last* operation before inclusion in the query. Otherwise there's no guarantee on the safety (and even if what you do *is* safe, it doesn't make it right if it's not obvious).
Also, some notes on your thoughts:
I said this is to filter a users' input before using it to query the database (a query does not necessarily mean inputing data into the database).
You can have SQL injection with both reading and writing queries.
Your particular function will break if you give it an array for example. For the url case you also maybe wanted VALIDATE in the first filtering.
Re: Input Sanitization
Posted: Mon Nov 21, 2011 4:58 pm
by Hermit TL
Thank you, Mordred. I appreciate you taking the time to help me out. I'm sure you're right about the unforeseen results from multiple functions being used on the same string but I was assuming (which was my primary mistake) that any unforseen/odd results would have been filtered to the point of being useless. I'll start by moving the mysql_real_escape_string() to the end of the function. I also understand your concerns about trying to create a 'Universal Filter' and while that may appear to be what I was doing; this function is only used in a limited manner; and by no means will I consider it a replacement for learning more about php security. I will surely run some extensive testing on it (after studying mysql injection) before I dare use it on a live site.
Thanks again!
EDIT:
For anyone reading this, thought you might want the modified function posted. (I have not yet run security tests on this.)
*DO NOT CUT & PASTE FOR YOUR SITE SECURITY*
Code: Select all
<?php
//****************************************
//****************************************
//** security.php by Hermit TL
//**
//** Version 0.3 Alpha
//**
//** Supports plain (english) text
//** and URL input only.
//**
//** Returns NULL on failure.
//****************************************
//****************************************
function security_filter ($input, $what) {
if (is_array($input)){ $input=NULL; }
switch ($what){
case "string":
switch (is_string($input)){
case true:
$input = filter_var($input, FILTER_SANITIZE_STRING, FILTER_NULL_ON_FAILURE);
break;
default:
$input = NULL;
break;
}
break;
case "url":
$chk = filter_var($input, FILTER_VALIDATE_URL, FILTER_NULL_ON_FAILURE);
switch($chk){
case !NULL:
$input = filter_var($input, FILTER_SANITIZE_URL, FILTER_NULL_ON_FAILURE);
break;
default:
$input = NULL;
break;
}
break;
default:
$input = NULL;
}
if ($input == false){
$input = NULL;
}
$input = mysql_real_escape_string($input);
return $input;
}
?>