How to escape SQL Injection?

[CrashOkami]

I've uploaded my newest version of the site, but I found out I can't post any kind of text that has an apostrophe in it, which is impossible, since I'm writing full articles. I've read topics about "my_sql_real_string()", but I can't figure out how to implement it to my existing code. Plus, I have no means of transforming my MySQL database into a MySQLi at the moment.

 

Since I get the error I've set to come when the query cannot be executed, I suppose this function has to be inside the query, right? So, can you please show me how to do it in the code below?

 

<?php

$Title = $_POST["Title"];
$Small_desc = $_POST["Small_desc"];
$Desc = $_POST["Desc"];
$Image = $_POST["Image"];
$Author = $_POST["Author"];

if(!$Title=="" && !$Small_desc=="" && !$Desc=="" && !$Image=="" && !$Author=="")
{

$db_name = "###";
$link = mysql_connect("###", "###","###") or die("There was an error connecting to the database.");
$select_db = mysql_select_db($db_name, $link);
$add_query="INSERT INTO news(News_ID, Title, Small_desc, Description, Image, Author, Date) values (NULL, '$Title', '$Small_desc', '$Desc', '$Image', '$Author', NOW())";
$add_results = mysql_query($add_query, $link) or die("Record not inserted. Could not execute query. Please try again. <br> <br>");
echo "Record successfully inserted. Go back to the <a href=admin_index.php>admin index page.</a>";
}

else
{
echo "Error, please fill in all the fields as needed. If the problem persists, contact the webmaster.";
}
?>

Where ### = my database/host details. Thank you once again! Although on most of my topics, I figure it out before someone posts the solution, I believe this will not be the case here.

[HDFilmMaker2112]

Try this:

 

$Title = mysql_real_escape_string($_POST["Title"]);
$Small_desc = mysql_real_escape_string($_POST["Small_desc"]);
$Desc = mysql_real_escape_string($_POST["Desc"]);
$Image = mysql_real_escape_string($_POST["Image"]);
$Author = mysql_real_escape_string($_POST["Author"]);

 

You should however look into moving to MySQLi (note the I) and using mysqli_real_escape_string.

[CrashOkami]

This is greatly appreciated, HDFilmMaker. Indeed, when my contract with this host ends (if not earlier), I will move on to a host with MySQLi. As noted under my profile picture, I'm a beginner, so I was unaware of this problem, and didn't bother searching for the differences between MySQL and MySQLi, so I didn't care if the host supported it or not. I'll try your code now and edit this post to let you know if it worked, thank you!

 

EDIT: now it points me to the ELSE error: "Error, please fill in all the fields as needed. If the problem persists, contact the webmaster.", although I've filled in all the fields as needed :/

[HDFilmMaker2112]

try changing the if statement to this:

 

if($Title!="" && $Small_desc!="" && $Desc!="" && $Image!="" && $Author!="")
{

 

The "!" before something like isset or empty works, but I think it may cause issues before a variable.

[CrashOkami]

Meh, still same, points me to the ELSE statement. Should I try rebuilding my whole database with MySQLi? Would it be worth it? At the very least, now would be the best time to do it, since it's empty of records, save for 1 admin account, and the tests which I'm always deleting.

[HDFilmMaker2112]

Meh, still same, points me to the ELSE statement. Should I try rebuilding my whole database with MySQLi? Would it be worth it? At the very least, now would be the best time to do it, since it's empty of records, save for 1 admin account, and the tests which I'm always deleting.

 

You shouldn't really have to rebuild anything. MySQLi is just the PHP to MySQL connection code. You still run MySQL  you're just connecting to it with PHP's improved MySQL code structure.

 

Basically just change mysql to mysqli in a lot of the functions, plus a few changes to the data in them.

 

Just add i to the connect function, add an i to mysql_select_db and flip the db_name and link variables, and add an i to mysql_query.

$link = mysqli_connect("###", "###","###") or die("There was an error connecting to the database.");
$select_db = mysqli_select_db($link, $db_name);
$add_query="INSERT INTO news(News_ID, Title, Small_desc, Description, Image, Author, Date) values (NULL, '$Title', '$Small_desc', '$Desc', '$Image', '$Author', NOW())";
$add_results = mysqli_query($add_query, $link) or die("Record not inserted. Could not execute query. Please try again. <br> <br>");
echo "Record successfully inserted. Go back to the <a href=admin_index.php>admin index page.</a>";
}

 

Here's the MySQLi overview on PHP.net: http://www.php.net/manual/en/mysqli.overview.php

 

And here's a list of the mysqli functions: http://www.php.net/manual/en/mysqli.summary.php

(A majority of what you're looking for is under the "Methods" table on that page.)

 

 

As to why the if statement isn't working, not entirely sure, somebody should be able to spot something.

[CrashOkami]

I can't thank you enough for all this, I will start the conversion process later. So, basically, wherever I spot a "MySQL", I change it to "MySQLi", and it's converted to MySQLi. Thank you again!

 

I suggest the mods leave this open so if anyone finds the error in the above code posts the solution to it.

[boompa]

So, basically, wherever I spot a "MySQL", I change it to "MySQLi", and it's converted to MySQLi.

 

No, there's more to it than that. You also need to convert your querying to use prepared statements.

[CrashOkami]

Yup, I meant that I need no programs etc, just alter the code, commands, functions and all that. Thanks for the clarification, though

[Pikachu2000]

So, basically, wherever I spot a "MySQL", I change it to "MySQLi", and it's converted to MySQLi.

 

No, there's more to it than that. You also need to convert your querying to use prepared statements.

 

Why? There's no requirement to use prepared statements to use the mysqli extension functions.

[boompa]

No, it's not a requirement. But if you want to avoid SQL injections, as is the case here, then it's certainly the recommended course.

[Pikachu2000]

Still, there should be problem no with SQL injection when not using prepared statements as long as the data is properly validated and escaped.

[HDFilmMaker2112]

@CrashOkami: Just to elaborate on what Pikachu2000 and boompa are discussing:

 

 

Prepared Statements are run this way (there may be syntax errors because I rarely use prepared statements, actually up to this point never in an actual site):

$Title = $_POST["Title"];
$Small_desc = $_POST["Small_desc"];
$Desc = $_POST["Desc"];
$Image = $_POST["Image"];
$Author = $_POST["Author"];

$stmt = mysqli_prepare($link, "INSERT INTO news (News_ID, Title, Small_desc, Description, Image, Author, Date) 
VALUES (?, ?, ?, ?, ?, ?, ?)");
mysqli_stmt_bind_param(NULL, $Title, $Small_desc, $Desc, $Image, $Author, NOW());
mysqli_stmt_execute($stmt);

 

Non-Prepared statements with SQL Injection Prevention are run this way:

$Title = mysqli_real_escape_string($link, $_POST["Title"]);
$Small_desc = mysqli_real_escape_string($link, $_POST["Small_desc"]);
$Desc = mysqli_real_escape_string($link, $_POST["Desc"]);
$Image = mysqli_real_escape_string($link, $_POST["Image"]);
$Author = mysqli_real_escape_string($link, $_POST["Author"]);

$add_query="INSERT INTO news(News_ID, Title, Small_desc, Description, Image, Author, Date) 
values (NULL, '$Title', '$Small_desc', '$Desc', '$Image', '$Author', NOW())";
mysqli_query($link, $add_query)

 

Both of the above essentially do the same thing.

 

Now here's the primary differences:

 

Prepared statements store the query with the variable insert values in memory (the "?" in the query are essentially variables/placeholders that get filled in by the "bind_param" function), so you can place the query in a loop, drop in values (via the "bind_param" function ), and have it run through the same query repeatedly with different insert values. When inserting data via the same query per single script execution, this is the preferred and faster method. It would be perfect for inserting data from an array or foreach statement, where you need to iterate through the array and run the query multiple times with the same table and column names but different values (essentially inserting different rows based on the same query).

 

This method should, IMO, be used sparingly, because...

 

When using prepared statements with a single set of values to insert (as shown in the above example), where you're not looping through an array or foreach statement, then prepared statements can run 7% to 15% slower than a straight mysqli query with mysqli_real_escape_string. I would say 90% of the time you're not going to be inserting, selecting, updating, or deleting multiple different rows based on the same query at the same time.

 

 

Know the tools available to you and use the right one for the given situation. Prepared statements are not the be-all-end-all of query statements; which some people will have you believe. If they were, PHP wouldn't have the other options available.

[cpd]

An article I recently read, linked to me by CrayonViolent in IRC, clearly explains why a variable run through mysql_real_escape_string isn't enough to secure your database. More experienced PHP programmers are likely aware a nere escape_string isn't enough but for the newbies I definitely recommend reading it.

 

Having a read of any referenced articles in this piece of text is also worthwhile as I've found stuff I was slightly sketchy on clearly explained. Whilst it was written back in 2007 5 years ago, I still feel its necessary today.

 

http://www.webappsec.org/projects/articles/091007.txt

[HDFilmMaker2112]

An article I recently read, linked to me by CrayonViolent in IRC, clearly explains why a variable run through mysql_real_escape_string isn't enough to secure your database. More experienced PHP programmers are likely aware a nere escape_string isn't enough but for the newbies I definitely recommend reading it.

 

Having a read of any referenced articles in this piece of text is also worthwhile as I've found stuff I was slightly sketchy on clearly explained. Whilst it was written back in 2007 5 years ago, I still feel its necessary today.

 

http://www.webappsec.org/projects/articles/091007.txt

 

And honestly I'd still take speed over 100% security. But if you follow the above article, you should get both, speed improvement and 100% security.

[HDFilmMaker2112]

Anyway, I will say that article did go back and make me add backticks, quote numbers, and use intval on all numbers (I'm even using it on inner-script generated number).

 

Here's the primary things you need to consider when using mysqli_real_escape_string:

 

 

1. Write properly quoted SQL:

        1.1. Single quotes around values (string literals and numbers)

        1.2. Backtick quotes around identifiers (databases, tables, columns, aliases)

    2. Properly escape the strings and numbers:

        2.1. mysql_real_escape_string() for all values (string literals and numbers)

        2.2. intval() for all number values and the numeric parameters of LIMIT

        2.3. Escape wildcard/regexp metacharacters

                (addcslashes('%_') for LIKE, and you better avoid REGEXP/RLIKE)

        2.4. If identifiers (columns, tables or databases) or keywords

                (such as ASC and DESC) are referenced in the script parameters,

                make sure (and force) their values are chosen only as one of an

                explicit set of options

        2.5. No matter what validation steps you take when processing the user

                input in your scripts, always do the escaping steps before

                issuing the query. Validation is not a substitute for escaping!

[cpd]

Using object orientation would actually provide speed as well. If you split your "validation" and "sanitisation" up into different areas of your application then re-use the code as is the way with OO, you will end up programming very quickly anyway.

 

 

[HDFilmMaker2112]

Using object orientation would actually provide speed as well. If you split your "validation" and "sanitisation" up into different areas of your application then re-use the code as is the way with OO, you will end up programming very quickly anyway.

 

 

 

Really? I've always heard OO runs slower than a regular function, and both run slower than regular code (except when running the same code more than once, but functions are supposedly faster than OO).

[cpd]

Object-orientation naturally has a larger overhead but in terms of your "rate of coding" you will be able to, theoretically, finish a program far quicker programming in an object-oriented style compared with a procedural style. How I see it is the foundations of an object-oriented system would probably take longer than a procedural system but the programming rate would increase exponentially, using object-orientation, as the application grows in size.

 

Until recently I never actually gave it much thought but after thinking about it logically, procedural style software could run quicker than object-oriented software due to the overhead involved - it pretty much comes down to the overhead.

 

As you can see you were referring to execution time, I was referring to development time.

[PFMaBiSmAd]

CrashOkami, the reason your code is going to the ELSE clause is because none of the variables being set from the $_POST data, are actually being set.

 

Since the connection to the database does not exist at the point where the mysql_real_escape_string statements are at, those statements are triggering a bunch of php errors and returning NULL values. This is also why you need to have php's error_reporting set to E_ALL and display_errors set to ON when you are trying to debug any code.

 

You need to make the database connection before calling any other mysql_ function.

 

The reason you cannot insert data with single-quotes in it, is because your code must escape all string data being put into a query statement OR use prepared sql statements (which are supported in mysqli and PDO) so that any special sql characters in the string data don't break the sql syntax (which also allows hackers to inject sql.) The purpose of the mysql_real_escape_string function is to escape string data being put into a query statement. When using mysql_ (no i), you must use mysql_real_escape_string on string data. When using mysqli_ (with the i) without using prepared statements, you must use mysqli_real_escape_string on string data. When using mysqli_ (with the i) with prepared statements or using PDO with prepared statements, you don't need to specifically escape string data being put into a query. For non-prepared statements, numerical data should be validated or cast as the appropriate numerical data type, since it is normally used in a query statement as a number and escaping it won't prevent sql injection.