SQL injection

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

thatsme
Forum Commoner
Posts: 87
Joined: Sat Apr 07, 2007 2:18 am

SQL injection

Post by thatsme »

I am trying to prevent sql injection.

Code

Code: Select all

function EscapeQuotes($data)
{
   if(get_magic_quotes_gpc())
   {
     $data=array_map(stripslashes, $data);
   }
   
   return array_map(mysql_real_escape_string, $data);    
}
 
// calling
   $post = EscapeQuotes($_POST);
    extract($post);
    $check_login_QUERY = mysql_query("SELECT * FROM members WHERE member_email_id='$email_id'  AND member_password='$password'");
 
 
If there is anything that has to be modified in the above code, please let me know.

Thanks
User avatar
jimthunderbird
Forum Contributor
Posts: 147
Joined: Tue Jul 04, 2006 3:59 am
Location: San Francisco, CA

Re: SQL injection

Post by jimthunderbird »

Your code is concise and elegant, guess it prevents most of the sql injection attempts. :D
User avatar
Zoxive
Forum Regular
Posts: 974
Joined: Fri Apr 01, 2005 4:37 pm
Location: Bay City, Michigan

Re: SQL injection

Post by Zoxive »

jimthunderbird wrote:Your code is concise and elegant, guess it prevents most of the sql injection attempts. :D
Escaping strings alone is not enough so prevent sql injection.

You need to verify, that it only contains the characters of which you allow.
Ex. Usernames: 5-16 Characters long, only Letters and numbers and underscores.

There are many, many other topics in these forums (and Internet!) about sql injection, and how to prevent it.
User avatar
jimthunderbird
Forum Contributor
Posts: 147
Joined: Tue Jul 04, 2006 3:59 am
Location: San Francisco, CA

Re: SQL injection

Post by jimthunderbird »

Yeah, those will be extra, like checking correct email format also...
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Re: SQL injection

Post by Mordred »

Zoxive wrote:Escaping strings alone is not enough so prevent sql injection.

You need to verify, that it only contains the characters of which you allow.
Ex. Usernames: 5-16 Characters long, only Letters and numbers and underscores.
While the first statement is generally true ( I wrote a paper on the subject: "The Unexpected SQL Injection"), it is not true in this context. Escaping strings is not intended as a security measure only, it is provided exactly so you can put any string you like in the data part of an SQL query. Thus the verifications you advise for are okay, but not necessary to prevent SQL injection. The escaping does that part.
jimthunderbird wrote:Your code is concise and elegant, guess it prevents most of the sql injection attempts. :D
No, the code is horrible, it uses functions that every PHP programmer should scratch in their head, and never ever use, and it uses a couple of wrong and dangerous misconceptions.

The EscapeQuotes() function is flawed, I've written a long rant about it, which I won't repeat here.

The other abomination is the use of extract(), as it injects into the global namespace any values passed in $_POST. Welcome back register_globals.

In short, this is a horrible way to write code, even if the small piece presented here is not vulnerable.
User avatar
Oren
DevNet Resident
Posts: 1640
Joined: Fri Apr 07, 2006 5:13 am
Location: Israel

Re: SQL injection

Post by Oren »

Mordred++
User avatar
s.dot
Tranquility In Moderation
Posts: 5001
Joined: Sun Feb 06, 2005 7:18 pm
Location: Indiana

Re: SQL injection

Post by s.dot »

Also, array_map() doesn't take into account nested arrays in your data. I'd make it a habbit to individually call mysql_real_escape_string() on everything you want escaped.

Moved to PHP security.
Set Search Time - A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.
thatsme
Forum Commoner
Posts: 87
Joined: Sat Apr 07, 2007 2:18 am

Re: SQL injection

Post by thatsme »

thanks for your valuble comments.

When i was searching in google about mysql_real_escape_string,

I saw the below code,

Code: Select all

 
<?php 
    /** 
    * PHP5 
    */ 
    function array_clean(&$value) 
    { 
        if (ini_get('magic_quotes_gpc')) { 
            $value = stripslashes($value); 
        } 
    } 
 
    array_walk_recursive($_GET, 'array_clean'); 
    array_walk_recursive($_POST, 'array_clean'); 
    array_walk_recursive($_COOKIE, 'array_clean'); 
?>
 
Can i use the above code safely?
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Re: SQL injection

Post by Mordred »

thatsme wrote:thanks for your valuble comments.
You're welcome. Now read them.
thatsme wrote:Can i use the above code safely?
Safe from what? From SQL injection - no.
thatsme
Forum Commoner
Posts: 87
Joined: Sat Apr 07, 2007 2:18 am

Re: SQL injection

Post by thatsme »

Mordred,

Can you suggest me some sites which guides me in preventing sql injection - I am not asking for ready made code. Just guidance.

Thanks
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Re: SQL injection

Post by Mordred »

In the article mentioned above (http://www.webappsec.org/projects/artic ... 7.shtml#pb) there is a collection of the major papers on the subject.
User avatar
hannnndy
Forum Contributor
Posts: 131
Joined: Sat Jan 12, 2008 2:09 am
Location: Iran>Tehran
Contact:

Re: SQL injection

Post by hannnndy »

please tell me what is sql injection? :D

im a biginner in security :oops:
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Re: SQL injection

Post by John Cartwright »

hannnndy wrote:please tell me what is sql injection? :D

im a biginner in security :oops:
Google is your friend > http://en.wikipedia.org/wiki/Sql_injection
Attilitus
Forum Commoner
Posts: 27
Joined: Wed Aug 08, 2007 2:32 pm

Re: SQL injection

Post by Attilitus »

I don't understand why more people do not implement their security into their database abstraction layer. It seems like a no brainer to me, it eliminates all possibility of sql exploitation. Escaping your data using any other means leaves room for clumsy errors, and oversights.

For example, Wordpress recently had an exploit caused by trackback pings in foreign charsets. They didn't escape data as part of their database abstraction layer, and when they converted the previously safe foreign charset into the local charset it contained unsafe information that was then added to the database.

That is a crazy example, but the point is this: Implement your SQL injection security on the database abstraction layer because then there will be absolutely NO chance of error.
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Re: SQL injection

Post by Mordred »

Damn right. Actually there is no other correct way of escaping except in the database layer -- it is a bug if you escape data for SQL without or before (or after) actually using it in a query.

I go one step further - I write no SQL code whatsoever - only through a generator.
Post Reply