0

I've been researching how to completely prevent against SQL injection, but there are so many different forms of injecting that completely preventing it looks impossible to me. I am using prepared statements and put up the following simple function to prevent against SQL injections. Does this secure against (the most) SQL injections, if I run this function on every posted variable? I have also read about multi-byte characters, how can I secure my queries against that?

function securevar($var)
{
    $var = str_replace("'", "'", $var); //secure '
    $var = str_replace('"', """, $var); //secure "
    return $var;
}
  • What database management system? – Gumbo Oct 26 '14 at 20:25
  • 2
    Your question is if this function in addition to prepared statement is sufficient, right? If it is, I think you should rephrase your title to reflect this, otherwise you might get a lot of answers telling you that the function by itself is not sufficient. About multi-byte charsets: if you use PDO and either big5, cp932, gb2312, bgk, or sjis, you might be interested in this post. – tim Oct 26 '14 at 21:57
  • 2
    misses select * from users where id = $id, so no – Filip Haglund Oct 26 '14 at 23:44
  • 2
    I'd say that it's not only useless, it's counterproductive. It alters original data - I assume it replaces quotes by HTML entities. So while it may work with HTML, it malforms other data types. Say you want to store JSON data in database, you would end in {"key": "value"}, which is not valid JSON at all. – el.pescado Oct 27 '14 at 8:49
  • 1
    What el pescado said, with the addition that if you don't notice that all your values are messed up, and they appear normal on the page, that means you're forgetting to HTML-escape properly when generating page output and you probably have lots of XSS vulnerabilities. Storing data HTML-encoded in the database is a grievous antipattern. – bobince Oct 27 '14 at 9:29
7

No, it doesn't, use prepared statements as is plus single query functions and proper input validation. The latter one is the default for most functions in PHP (e.g. mysqli::query Vs. mysqli::multi_query).

Prepared Statements

Prepared statements will forward the data of a variable and its type separately to your DBMS which in turn will either escape all offending characters on your behalf or parse the data independent of the actual query (depending on the DBMS). There's nothing you have to know about which characters need to be escaped and which don't. This is the most secure way to achieve this.

<?php

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
$mysqli = new \mysqli(null, null, null, "database");
$mysqli->prepare("SELECT * FROM `users` WHERE `id` = ?");
$mysqli->bind_param("d", $_GET["id"]);
$mysqli->execute();
// ...

All of this complexity can be abstracted, have a look at e.g. Drupal source code.

Single Query Functions

Single query functions ensure that a query of yours will always only yield a single query that will be passed to the DBMS. Effectively killing any possibility that somebody ends your query and executes another; a typical scenario with SQL injection, example:

<?php

$_GET["id"] = "1;SET foreign_key_checks=0;DROP TABLE users";

$mysqli->multi_query("SELECT * FROM `users` WHERE `id` = {$_GET["id"]}");

Validation

This is perhaps the most important advise and is always important, no matter what you try to achieve. Use PHP's built-in filter_* functions and check other big project's for validating stuff that isn't covered by these functions.

Combined Example

Let's put it all together.

<?php

// max_range default is PHP_INT_MAX.
$userId = filter_input(INPUT_GET, "id", FILTER_VALIDATE_INT, array(
  "options" => array("min_range" => 1),
));

// Contains either a valid integer now or either FALSE or NULL.
if ($userId === null || $userId === false) {
//if (!$userId) {
  throw new \InvalidArgumentException();
}

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
$mysqli = new \mysqli(null, null, null, "database");
$stmt = $mysqli->prepare("SELECT `name` FROM `users` WHERE `id` = ?");
%stmt->bind_param("d", $userId);
$stmt->execute();
$stmt->fetch($userName);
$stmt->close();
$mysqli->close();

exit("Hello {$userName} with {$userId}.");
  • Have a look at this question. Prepared statements are not 100% safe according to the answer. stackoverflow.com/questions/134099/… – P.Yntema Oct 27 '14 at 6:45
  • 1
    "DBMS [...] will escape all offending characters on your behalf" It's worth noting that in most cases, using parametrized queries don't involve escaping at all, as it eliminates parameter interpolation in query string altogether. Besides security, it also increases performance. The most notable exception is MySQL, although it can be configured to perform real prepared statements. – el.pescado Oct 27 '14 at 8:11
  • Yes, if you do it wrong. Still better than concatenating strings containing user input. – Axel Oct 27 '14 at 8:11
  • 2nd order injection happens if you don't honor another essential rule, always validate your input. I'll extend my answer. PHP and MySQL feature real prepare statements via the \mysqli extension and isn't simply escaping. I'll extend my answer on this as well. Thanks for your input guys. – Fleshgrinder Oct 27 '14 at 10:34
  • This looks good and clear to me. In your "Single Query Functions" part you recommend to use Single Queries, but where can I find that part back in your combined example? Or do prepared statements automatically only allow one query to be executed? – P.Yntema Oct 27 '14 at 16:28
14

No. SQL Injection should be prevented by parameterizing your queries.

OWASP has some great resources to help you with this.

Here is one such sheet to show this in several languages. The PHP cheat sheet also has some more information regarding sql injection.

To directly address your hard coded filter function check out this source demonstrating the exploitation of hard coded filters, including quotes and single quotes.

  • While I agree with you, your source demostrating the exploitation doesn't apply to OP's example: the OP isn't rejecting inputs based on their code, it is only quoting comment strings in the input. – Pablo Oct 27 '14 at 8:35
1

As @Douglas and @fleshgrinder said before, no.

Consider the following request:

SELECT title, news FROM news_table WHERE id = $id

Applied for $id = 5 is fine, now if $id = 5 OR 1=1 will display all the news... Not a big deal?

Now if $id = 5 UNION SELECT login as title, password as news from secret_credentials_table where is_admin = 1 should also be a valid SQL request, with no quotes ...

  • Use prepared Requests
  • Typed arguments also helps (but is not the silver bullet either!)

For the typed argument, if you ensure that your $id is an integer, then such an injection is impossible. But of course, some request expect string as input, so you lose all the benefits of this.

  • So if I understand you correctly, prepared statements are completely safe against SQL injection? What if I would add the equal sign to my function? – P.Yntema Oct 27 '14 at 9:28
  • 1
    It's the wrong way to approach the problem, there are so many characters that you would have to filter (and those characters might be present in a legitimate request). If you filter '=', the same injection can just be replaced by $id = 5 OR 1 IN (SELECT id FROM table) without any quotes or = or any forbidden char really. – Dillinur Oct 27 '14 at 12:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.