4

I have the following PHP code on an intranet site, and I've recently learned about a Firefox addon 'SQL Inject Me'. Out of curiousity I've tried to run it on this really simple site (basically a phonebook with an admin account).

The test returned 51 #302 errors, but when I tried them I could not do any harm nor access (to) the database.

Is there any more to do than this to prevent the injection? When I've searched, they all advised PDO prepared statements, but this was done using that.

PHP

include_once("inetcon.php");
session_start();
$uid = $_POST['uid'];
$pass = $_POST['pass'];

// This is what I've added after seeing the errors

$uid = mysql_real_escape_string($uid);
$pass = mysql_real_escape_string($pass);

$uid = str_replace("'","fag",$uid); 

// end of block

$extract = $handler->prepare('SELECT * FROM net_users WHERE users_fname = ? and users_role = ? and users_active = "2"');
$extract->execute(array($uid, md5($pass)));

The addon returns all the basic ones like:

Server Status Code: 302 Found
Tested value: 1' OR '1'='1

Thanks for the help.

2
  • If you escape and use prepared statements, you're basically corrupting data. Doesn't the add-on report provide any information apart from "not good"? Commented Sep 8, 2014 at 9:28
  • 2
    mysql_real_escape_string does not modify the subject. it returns escaped string. So you're not doing anything by this. But using prepared statements is sufficient here. Though as a general rule of thumb - it is good idea to check input format/validity.
    – poncha
    Commented Sep 8, 2014 at 9:29

2 Answers 2

4

This:

    $uid = $_POST['uid'];
    $pass = $_POST['pass'];

Should be:

$uid = mysqli_real_escape_string($db_connect, $_POST['uid']);
$pass = mysqli_real_escape_string($db_connect, $_POST['pass']);

This way the variables become the escaped string else you just don't do anything to them. And why do you use mysql_* with mysqli_* or pdo? Using prepared statements is good tough.

UPDATE You could also add trim() to remove useless spaces and strip_tags() to remove all html tags:

$uid = mysqli_real_escape_string($db_connect, trim(strip_tags($_POST['uid'])));
$pass = mysqli_real_escape_string($db_connect, trim(strip_tags($_POST['pass'])));

so an input like: <b onClick="some javascript injection">Test</b> is this <u>striped</u>? will become: Test is this striped?

2
  • 1
    I'm tired of seeing strip_tags() suggested as a sanitisation tool. Are end users supposed to learn HTML so they can reword everything they type to skip angle brackets? Commented Sep 8, 2014 at 9:49
  • @ÁlvaroG.Vicario Like I said You could. And as I showed it could get rid of some unwanted data. I once had a WordPress (which got hacked) site where a javscript function was activated with a click or hover to get you to a malware infected website.
    – SuperDJ
    Commented Sep 8, 2014 at 10:00
0

Well, yeah...

Just found this answer, so this question is a duplicate in fact:

How to fix Server Status Code: 302 Found by SQL Inject Me Firefox Addon

What they say here is that this doesn't mean that the form is not protected it is a sign of redirection:

302 is the server's way of saying "I want you to go to [somewhere else]" (in this case login.php). It is not an error but a perfectly normal response.

By the rest of the code it makes perfect sense, as it really does a header location after the query - even if it was successful or has failed (by looking at this code it seems a bit lame to include the redirection in the while clause and also right after it, but meh...)

while ($results = $extract->fetch()) {
    $_SESSION['username'] = $results['users_lname'];
    echo $_SESSION['username'];
    header('location: ../phonebook.php');
}
header('location: ../phonebook.php');

Anyway, thanks anyone for the help

0

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