SQL injection Hysteria? Or not?

[StevenOliver]

After spending months learning how to replace my simple '90's PHP code with all the modern "hacker resistant, SQL-injection-resistant" code (like mySQLi prepared statements, sanitizing, confusing $db-> arrow OOP stuff, etc.) I can't help but wonder how this modern style is any safer than my old code?

Here is a snippet of my old code. "Register Globals" is off, and my preg_replace limits all user input to just letters and numbers... what could possibly go wrong?

 

<?php
$sanitized = preg_replace('/[^\da-zA-Z]/i', '',$_POST["customer_input"]);
if(isset($submit)) {
$statement = "insert into myTable (customer_data) values ('$posted_values')";
mysql_query($statement,$link);
}

I am enjoying the learning, and the challenges... but sometimes I wonder if my websites are so simple that perhaps I don't need all the fancy new code.

[benanamen]

28 minutes ago, StevenOliver said:

what could possibly go wrong? 

Run your code in Php 7 and see what happens.

[mac_gyver]

firstly, you should NOT modify/sanitize user input, except perhaps to trim it. by modifying it, you are changing the meaning of it. (a few versions ago of this forum software, the email addresses were 'sanitized' inconsistently, resulting in multiple values mapping to the same account and a hacker was able to do a password recovery for an admin using the hacker's email address, which allowed the hacker to log in as that admin and all the user's data was copied.) what you should do instead is to just validate the user input, to make sure it is suitable within the context of your application. if the data is not valid, don't use it at all and output a message to the user telling them what was wrong with the submitted data.

next, the regex pattern you have in your example isn't what you will use for all possible user input. things like names, text content (like we are typing here in the forum) , or email addresses require other characters like single-quotes or strings that are hexadecimal  (0-9 and a-f) values. as soon as you allow these and put them directly into the sql query statement, you are open to sql injection.

when you get to the database layer, all you should be concerned with is protecting against sql special characters in the data from breaking the sql query syntax (which is how sql injection is accomplished.) you should not care about where the data came from, what it means, or what type of validation/sanitizing it may or may not have received prior to that point. the simplest , most general-purpose, and surest way of providing this protection is to use a prepared query (assuming you are using the much simpler php PDO extension. the php mysqli prepared query programming interface is overly complicated and inconsistent, resulting in either a lot of extra typing for each query or in a bunch of slow code if you write a single-point general-purpose prepared query function/method.)

another advantage to using prepared queries are for the few cases where you need to execute the same sql query multiple times within an instance of your script. by preparing the query once, then just executing it with different sets of input data, you will save about 5% of the execution time for most, straight-forward, queries.

Edited November 1, 2018 by mac_gyver

[Phi11W]

7 hours ago, StevenOliver said:

After spending months learning how to replace my simple '90's PHP code with all the modern "hacker resistant, SQL-injection-resistant" code ... I can't help but wonder how this modern style is any safer than my old code?

Here is a snippet of my old code ... what could possibly go wrong?

<?php
$sanitized = preg_replace('/[^\da-zA-Z]/i', '',$_POST["customer_input"]);
if(isset($submit)) {
$statement = "insert into myTable (customer_data) values ('$posted_values')";
mysql_query($statement,$link);
}

 

Obligatory XKCD Reference - Little Bobby Tables

Regards,   Phill  W.

[StevenOliver]

9 hours ago, benanamen said:

Run your code in Php 7 and see what happens.

Benamen, sorry I left several lines of code out (figured the experts here didn't want to read lines and lines of my 90's code ?). I just posted a couple lines of my "old fashioned code" to ask if someone could come up with an example of how it could be misappropriated.

I guess an ulterior motive for my post was to have some examples of how my old code is bad -- that would give me the reassurance and "impetus" to keep on with the hours and hours of recoding (and learning how to) recode everything. Thank you.

[StevenOliver]

8 hours ago, mac_gyver said:

...what you should do instead is to just validate the user input

...another advantage to using prepared queries are for the few cases where you need to execute the same sql query multiple times

Really nice answer! Thank you! Using your answer, I have just now learned how to validate email. I don't know if it will be a "performance hit" but I'm using the "function domain_exists($email, $record = 'MX')" method to determine if emails like "ernie@hotmailsslslslslslss.com" is really valid.

The hardest part, and I don't want to bore everyone with my code, is my entire "Order Page" submits to itself -- I wish when the website was originally coded in the 90's it would submit/post to a final "thankyou.php" page.

What I mean is customer places order on "order.php" page, clicks Submit, then lands on "order.php" page again and sees "Thank you for your order!"

I wish customer places order on "order.php" page, clicks Submit, then lands on "thankyou.php" page to see "Thank you for your order."

(I have the knowhow to do this, but will take a buddhist-like calm mind which I don't have right now :-)

[StevenOliver]

2 hours ago, Phi11W said:

Obligatory XKCD Reference - Little Bobby Tables

Cute!

Anecdotally, years ago my own server had "Register Globals" turned on -- and when I heard about "SQL Injection," I tried every which way to screw up my own server. Never could make it work. (But I kept hearing Little Bobby Tales stories.)

[cyberRobot]

For what it's worth, PHP has a built in filter for checking email addresses. More information can be found here:
http://php.net/manual/en/filter.examples.validation.php

Instead of changing the value of the user response, you could display an error message when you detect something that shouldn't be there. If the user fills out an online form and forgets to include the @ symbol in their email address, for example, you could re-display the form with an error message about the email being incorrect. The user would then be given the chance to fix their mistake.

[cyberRobot]

2 hours ago, StevenOliver said:

I guess an ulterior motive...

Just to clarify benanamen's post, the mysql_* functions were removed from PHP 7. The code example you posted contains a call to mysql_query(), which no longer works in PHP 7 or later. Perhaps you're already aware of the change since you also mentioned that you're experimenting with MySQLi. Well, if you weren't aware, you are now. ?

[StevenOliver]

12 hours ago, cyberRobot said:

Just to clarify benanamen's post, the mysql_* functions were removed from PHP 7. ?

cyberRobot, thank you! I now understand what Benamen meant. I think I have learned more in the past couple weeks in this forum than I've learned in years. I feel like an elementary school kid suddenly doing immersion learning w/ Harvard grads! As long as I stick it out (and don't go running home crying) I think I'll learn a ton of stuff right here! 

[gizmola]

In a nutshell -- in the olden days you needed to be concerned about "escaping" your SQL strings.  Why?

Because anything that interfered with the parsing of the SQL statement, which I'm sure you know is as simple as a string that contains quotes in it, would break the SQL statement.

 

So as simple as:

 

$username = "admin";
$password = "' OR 1=1 OR 'password' = '";
$sql = "SELECT * FROM user WHERE username = '$username' AND password = '$password'";

 

This issue was addressed by functions like addslashes, mysql_escape_string and mysql_real_escape_string.  What this indicated clearly was that dealing with input and the potential of quotes and in some databases, characters that allowed one to batch multiple SQL statements together in one statement, was a potentially complicated and highly dangerous mechanic.

For this reason alone,  using prepared statements are beneficial, because you no longer have to use any string escaping functions.  

The other benefit is that these SQL injection attacks are no longer possible when you use prepared statements.