PHP Developers Network

A community of PHP developers offering assistance, advice, discussion, and friendship.
 
Loading
It is currently Wed Oct 16, 2019 3:23 am

All times are UTC - 5 hours




Post new topic Reply to topic  [ 11 posts ] 
Author Message
 Post subject: Input Sanitization
PostPosted: Mon Nov 21, 2011 1:40 am 
Offline
Forum Commoner

Joined: Mon Nov 21, 2011 1:16 am
Posts: 69
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.

Syntax: [ Download ] [ Hide ]
<?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;
}
?>


Top
 Profile  
 
 Post subject: Re: Input Sanitization
PostPosted: Mon Nov 21, 2011 4:15 am 
Offline
Forum Contributor

Joined: Sat Oct 22, 2011 10:50 am
Posts: 159
I'd do it something like this myself:


Syntax: [ Download ] [ Hide ]
<?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.
        }
    }
?>
 


Top
 Profile  
 
 Post subject: Re: Input Sanitization
PostPosted: Mon Nov 21, 2011 4:30 am 
Offline
Forum Commoner

Joined: Mon Nov 21, 2011 1:16 am
Posts: 69
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?


Top
 Profile  
 
 Post subject: Re: Input Sanitization
PostPosted: Mon Nov 21, 2011 4:31 am 
Offline
DevNet Resident
User avatar

Joined: Sun Sep 03, 2006 5:19 am
Posts: 1579
Location: Sofia, Bulgaria
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.


Top
 Profile  
 
 Post subject: Re: Input Sanitization
PostPosted: Mon Nov 21, 2011 4:48 am 
Offline
Forum Commoner

Joined: Mon Nov 21, 2011 1:16 am
Posts: 69
@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.


Top
 Profile  
 
 Post subject: Re: Input Sanitization
PostPosted: Mon Nov 21, 2011 5:55 am 
Offline
DevNet Resident
User avatar

Joined: Sun Sep 03, 2006 5:19 am
Posts: 1579
Location: Sofia, Bulgaria
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)


Top
 Profile  
 
 Post subject: Re: Input Sanitization
PostPosted: Mon Nov 21, 2011 6:26 am 
Offline
Forum Commoner

Joined: Mon Nov 21, 2011 1:16 am
Posts: 69
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:
Syntax: [ Download ] [ Hide ]
<?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;
}
?>


Top
 Profile  
 
 Post subject: Re: Input Sanitization
PostPosted: Mon Nov 21, 2011 8:45 am 
Offline
DevNet Resident
User avatar

Joined: Sun Sep 03, 2006 5:19 am
Posts: 1579
Location: Sofia, Bulgaria
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.


Top
 Profile  
 
 Post subject: Re: Input Sanitization
PostPosted: Mon Nov 21, 2011 9:25 am 
Offline
Forum Commoner

Joined: Mon Nov 21, 2011 1:16 am
Posts: 69
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.


Top
 Profile  
 
 Post subject: Re: Input Sanitization
PostPosted: Mon Nov 21, 2011 11:20 am 
Offline
DevNet Resident
User avatar

Joined: Sun Sep 03, 2006 5:19 am
Posts: 1579
Location: Sofia, Bulgaria


Top
 Profile  
 
 Post subject: Re: Input Sanitization
PostPosted: Mon Nov 21, 2011 5:58 pm 
Offline
Forum Commoner

Joined: Mon Nov 21, 2011 1:16 am
Posts: 69
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*
Syntax: [ Download ] [ Hide ]
<?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;
}
?>


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 11 posts ] 

All times are UTC - 5 hours


Who is online

Users browsing this forum: Majestic-12 [Bot] and 1 guest


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Jump to:  
Powered by phpBB® Forum Software © phpBB Group