How do I protect my website from sql injections?

Question

I have a basic website in PHP that is a search engine and run's off a mysql database but I need to protect it from sql injections can someone explain to me how I can do this?

My PHP script:

<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>Search Engine - Search</title>
</head>
<body>

<h2>Search Engine</h2>
<form action='./search.php' method='get'>
<input type='text' name='k' size='50' value='<?php echo $_GET['k']; ?>' />
<input type='submit' value='Search'>
</form>
<hr />
<?php
$k = $_GET['k'];

$terms = explode (" ", $k);
$query = "SELECT * FROM search WHERE ";

foreach ($terms as $each) {
$i++;

if ($i == 1)
$query .= "keywords LIKE '%$each%' ";
else
$query .= "OR keywords LIKE '%$each%' ";
}

// connect
mysql_connect("*******", "******", "*******");
mysql_select_db(*******);

$query = mysql_query("$query");
$numrows = mysql_num_rows($query);
if ($numrows > 0) {

while ($row = mysql_fetch_assoc($query)) {
$id = $row['id'];
$title = $row['title'];
$description = $row['description'];
$keywords = $row['keywords'];
$link = $row['link'];

echo "<h2><a href='$link'>$title</a></h2>
$description<br /><br />";
}

}
else
echo "No results found for \"<b>$k</b>\"";

// disconnect
mysql_close();


?>
</body>
</html>

http://stackoverflow.com/questions/60174/best-way-to-stop-sql-injection-in-php — Mat, Jan 22 '12 at 20:45
it's a common question,you should read an article something like this:http://www.learnphponline.com/security/sql-injection-prevention-mysql-php — Oyeme, Jan 22 '12 at 20:45
@bummzack - Funny that wbesite got it all wrong as well. Sanitizing input does not have to do with doing it yourself. — Christian, Jan 22 '12 at 20:50
@Oyeme An incredible wrong article. The very code snippet that claimed to be a silver bullet in stopping injection, **actually allows an injection**. These people have not a faintest idea of injections. Please, do not link that site again. — Your Common Sense, Jan 23 '12 at 07:23

score 1 · Answer 1 · answered Jan 22 '12 at 20:47

1

The PHP documentation has an article about this, see here.

Using the mysqli library over mysql is an even better idea, since it supports prepared statements and does the escaping mostly for you.

answered Jan 22 '12 at 20:47

fivedigit

18,464
6
54
58

yannis · Answer 2 · 2012-01-23T04:12:15.613

0

As always, the manual is a very good place to start. Read the article, it includes various examples and relevant techniques.

The gist of it is that you should never trust user input, and user input includes everything that comes from the environment, including $_GET, $_POST, $_COOKIE, $_SESSION, even `$_SERVER.

In your code you blindly trust:

$k = $_GET['k'];

and use its contents as is in your queries:

if ($i == 1)
$query .= "keywords LIKE '%$each%' ";
else
$query .= "OR keywords LIKE '%$each%' ";
}

Never do that again.

You need to validate that user input is what you expect it to be, and sanitize it before you use it in SQL queries. You can easily do both with the Filter functions.

Additionally you should really forget about mysql_* functions, they are essentially deprecated, the extension's development status is maintenance only and long term deprecation was announced recently. Instead you should move on to the mysqli_* functions (notice the i at the end? it stands for improved).

But I would suggest you skip those as well, and start using PDO and prepared statements. PDO offers a database agnostic object oriented interface and prepared statements sanitize your variables for you.

That's about it.

edited Jan 23 '12 at 04:12

answered Jan 22 '12 at 20:45

yannis

6,215
5
39
49

2

That doesn't even explain what SQL injection is, let alone offer a solution. – Christian Jan 22 '12 at 20:48
@ChristianSciberras That's why I left a comment saying I was writing a proper answer... Give it a read and tell me what you think. – yannis Jan 22 '12 at 20:59
Much better, but as I said below, PDO and mysqli are a matter of convenience. Much preferred, but not essential. By the way, mysqli supports prepared statements. – Christian Jan 22 '12 at 21:03
@ChristianSciberras Yes it does. But part of my answer is to skip mysqli alltogether and move on to PDO. – yannis Jan 22 '12 at 21:05
thanks (Im going to try and use PDO then) – Joe Jan 22 '12 at 21:11
@JoeGrimes Glad I could help. When you have some non trivial working code around PDO, you should post it for review at [Code Review Stack Exchange](http://codereview.stackexchange.com/), Stack Overflow's sister site for peer reviews... – yannis Jan 22 '12 at 21:14
@YannisRizos - The only difference between mysqli and PDO is that PDO offers more SQL drivers (rather than target mysql only). **JoeGrimes** good luck try to get PDO to work in two lines of code. **:)** – Christian Jan 22 '12 at 21:17
Angry? I pity your ignorance. :) – Christian Jan 22 '12 at 21:36
although `you should never trust user input` statement is true in general, it is complete offtopic here. `You need to validate user input and sanitize it` is one of most evil delusions of PHP community. – Your Common Sense Jan 23 '12 at 04:16
@Col.Shrapnel Care to elaborate? – yannis Jan 23 '12 at 04:17
If you bother to think the matter, you would realise that dynamical SQL composing should be absolutely irrelevant to data source. What if you are using data not from user input? Forget all sanitizing or what? – Your Common Sense Jan 23 '12 at 04:18
@Col.Shrapnel And how's that relevant to the question? I can accept that you consider some of the answer off topic, but shouldn't the same criteria apply to the rest of it? – yannis Jan 23 '12 at 04:21
The question is "how to protect from injection". "validating user input" is just not enough for this. SQL creation rules should be irrelevant of the data source, so, all the user input talk is offtopic here, deceiving one. Christian takes it right - all the rules should be destination-specific, not source. – Your Common Sense Jan 23 '12 at 04:25
@Col.Shrapnel Right. Again I was answering the question, which presented a specific piece of code, wasn't writing a treatise. Instead of reiterating stock advice, I linked to the manual page on SQL injection, which presents the vulnerability & avoidance techniques sufficiently (imho). – yannis Jan 23 '12 at 04:38
okay, you don't want to listen. I can't help it. – Your Common Sense Jan 23 '12 at 04:50
@Col.Shrapnel Oh I really want to listen. Is the tone I don't appreciate. But thanks, anyway. – yannis Jan 23 '12 at 04:54

Christian · Answer 3 · 2012-01-22T21:18:34.553

0

It's very easy to avoid SQL Injection.

Why?

The current trend to fix sql injection seems to point at magical buzzwords purporting to be a solution. This does not fix the real problem, the lack of knowledge by the developer writing the code.

Imagine a company selling a product. They most certainly do not give the product directly without any packing. Instead, they pass it on to a packing department.

Same in this case. You don't give the database the raw product, you need to pack it up first.

Why is this important? It's important because prepared statements and whatnot will not be available when you want to "output a PHP variable in HTML" ("or javascript"). Yes it is equally relevant to the issue at hand. The developer needs to learn to 'pack' the information correctly (mysql_real_escape_string, htmlspecialchars, json_encode), then, he might as well learn about better packing systems (PDO, template engines...).

Direct Queries

Let's look at your code:

if ($i == 1)
    $query .= "keywords LIKE '%$each%' ";
else
    $query .= "OR keywords LIKE '%$each%' ";

Simply make sure $each is actually encoded before putting it into the query, like so:

$each = mysql_real_escape_string( $each );

if ($i == 1)
    $query .= "keywords LIKE '%$each%' ";
else
    $query .= "OR keywords LIKE '%$each%' ";

Easy, no?

Parameterized Queries

These tend to do the encoding by themselves. It's a better system in general, but if you think your SQL Injection woes are over, think again.

Parameterized queries take your data as parameters instead of a direct query, and they encode that input by themselves.

But in truth, you can, either by mistake, or by necessity, still pass data directly to a query.

If you switch over to PDO, just as advised above, without knowing the consequences, you might as well have SQL injections and still don't know about it. PDO does nothing to stop you from running SQL queries directly, hence the same mistake can be ported there.

In such a case, either encode the input as advised above (PDO has such functionality as well).

edited Jan 22 '12 at 21:18

answered Jan 22 '12 at 20:56

Christian

27,509
17
111
155

1

`mysql_real_escape_string` is really bad advice in 2012, `mysql_*` have been essentially deprecated a long time ago... – yannis Jan 22 '12 at 21:03
thanks (I think im beginning to understand it) – Joe Jan 22 '12 at 21:03
@YannisRizos - That function is built on top of the same code of mysqli. If it's vulnerable, mysqli as well as prepared statements would be vulnerable since they all rely on that functionality. `mysql_` is a complete library without any need of upgrades, hence why development has stopped. It is just as capable as non-parameterized mysqli. Besides, your comment is like say 'batch/bash script files are bad advice in 2012'. Just because they're old and proven doesn't mean they're bad. – Christian Jan 22 '12 at 21:12
1

@ChristianSciberras Actually `mysql_*` development stopped because it was [badly written and became impossible to maintain](http://forge.mysql.com/wiki/Converting_to_MySQLi). `mysqli_*` was a from scratch rewrite, and its source is amazing compared to `mysql_*`. Please do some research before editorializing. – yannis Jan 22 '12 at 21:26
Funny the article you linked to said *'difficult to maintain'* rather than *'impossible'*. Perhaps you should do some research instead of relying on **cargo cult programming**. If ext/mysql was actually vulnerable, modern CMSes would have long stopped support for it - which is far from the truth. – Christian Jan 22 '12 at 21:33
@YannisRizos care to provide a link to "essential deprecation" that has been done "long time ago"? – Your Common Sense Jan 23 '12 at 03:59
@Col.Shrapnel You mean other than [the](http://www.php.net/manual/en/mysqlinfo.api.choosing.php) [manual](http://www.php.net/manual/en/mysqli.overview.php)? Development status is maintenance only, and long term deprecation has been announced... (and I'm going back to my answer to add that, thanks for reminding me) – yannis Jan 23 '12 at 04:06
`prepared statements would be vulnerable` is partially true about PDO and not true at all for the mysqli. native prepared statements do not use escaping, it's different mechanism. However, there are no know vulnerabilities in escaping, beside some ridiqulous encoding issues. – Your Common Sense Jan 23 '12 at 04:08
@YannisRizos deprecation is a term with certain meaning in PHP, and mysql still **not** deprecated, though it was *recently* (not "long time ago") has been proposed. – Your Common Sense Jan 23 '12 at 04:12
@Col.Shrapnel mysql_* was officially discouraged ever since mysqli_* appeared, and although the announcement for [soft deprecation was indeed recent](http://news.php.net/php.internals/53799), I consider it essentially deprecated a long time ago. – yannis Jan 23 '12 at 04:15
also note that your code, if pasted in the original script, would produce an error. – Your Common Sense Jan 23 '12 at 07:12

How do I protect my website from sql injections?

3 Answers3

Why?

Direct Queries

Parameterized Queries