CODEWITHSUNDEEP
phpmysqlsecuritysql-injection
csm232s
csm232s

Reputation: 1660

GET parameters vulnerable to SQL Injection - PHP

I've been asked to handle a security issue for a site which was set up by another programmer. As of yet, I haven't seen any of the code, so I'm going off of assumptions at this point and I want to cover my bases. The group hosting the site ran a security check and found that they had code vulnerable to SQL injection.

Example: www.example.com/code.php?pid=2&ID=35 (GET parameter ID is vulnerable to SQL Injection)

Now, because I'm a novice, I've explained that I can likely resolve the issue with the host, but their site would still need to be looked over by someone who has a deeper knowledge of security.

So, to take care of potential SQL Injections (and without seeing the code), I would use mysql_real_escape_string:

$query = sprintf("SELECT * FROM table WHERE pid='%s' AND ID='%s'",
            mysql_real_escape_string($pid),
            mysql_real_escape_string($id));

Additionally, I would consider mysqli_real_escape_string and prepared statements, but I don't know how they're configured. But would mysql_real_escape_string take care of potential SQL Injection?

Upvotes: 3

Views: 7125

Answers (5)

alex
alex

Reputation: 490423

Skip the old mysql_* stuff if you can and use PDO.

$pdo = new PDO('mysql:host=localhost;dbname=whatever', $username, $password);

$statement = $pdo->prepare('SELECT * FROM table WHERE pid=:pid AND ID=:id');

$statement->bindParam(':pid', $_GET['pid']);

$statement->bindParam(':id', $_GET['id']);

$results = $statement->execute();

var_dump($results->fetchAll());

Upvotes: 5

Matthieu Napoli
Matthieu Napoli

Reputation: 49633

If ID and PID are integer fields, why not casting them to int.

That way, you are sure to have a number, an no SQL injection :

$pid = (int) $pid;
$id = (int) $id;

Upvotes: 1

Aaria Carter-Weir
Aaria Carter-Weir

Reputation: 1679

That should be fine, but i'd always recommend using prepared statements.

Upvotes: 0

David Houde
David Houde

Reputation: 4778

Yes, mysql_real_escape_string() will escape any potentially hazardous characters. If you know that the arguments are numeric, it would not hurt to verify this aswell using is_numeric()

You should also look at mysql::prepare -- This will ensure that only 1 statement is executed, and prevent additional SQL vulnerabilities.

Upvotes: 1

fin1te
fin1te

Reputation: 4351

That function should be fine - your variables are inside single quotes in the SQL statement, and any single, or double quotes will be escaped.

This means that none of the variables can "break out" of the statement.

Upvotes: 2

Related Questions