I've tried SQL injecting my code which 'technically' should be vulnerable but it wont work

Question

I keep getting told my code is vulnerable to SQL injection, however I have since converted to mysqli extensions from mysql, and I've tried SQL injection attacks on myself but none of them seem to work so my question is...

Is my code actually secure, and if not, why wont the SQL injection work?

<?php

session_start();
if (!isset($_SESSION["email"])){
    header ("location: logout.php");
    die();
}


include('connect-db.php');

if (mysqli_connect_errno())
  {
  echo "Failed to connect to mysqli: " . mysqli_connect_error();
  }
else 
{ 

}

function newUser()
{


    $forename = $_POST['forename'];
    $surname = $_POST['surname'];
    $email = $_POST['email'];
    $securityq = $_POST['securityq'];
    $securitya = $_POST['securitya'];
    $password = ($_POST['password']);

    $query = "INSERT INTO admin (forename,surname,email,securityq, securitya,password) VALUES ('$forename','$surname','$email','$securityq','$securitya','$password')";

    include('connect-db.php');
    $data = mysqli_query ($db, $query)or die(mysqli_error($db));
    if($data)
        {

    }

}


function SignUp()
{
    if(!empty($_POST['email']))
    {
        include('connect-db.php');
    $query = mysqli_query  ($db, "SELECT * FROM admin WHERE email = '$_POST[email]'")
        or die(mysqli_error());
        if(!$row = mysqli_fetch_array($query) or die(mysqli_error()))
        {
            newuser();
            echo ("<SCRIPT LANGUAGE='JavaScript'>
    window.alert('Admin Registration Successful')
    window.location.href='adminhome.php';
    </SCRIPT>");

        } 
        else
        {
            echo ("<SCRIPT LANGUAGE='JavaScript'>
            window.alert('Sorry You are already a registered user!')
            window.location.href='adminhome.php';
    </SCRIPT>");


        }
    }

}
if(isset($_POST['submit']))
    {
    SignUp();
}

?>

The error I get upon attempted SQL injection are all similar to this one:

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'DROP table pdf',','lll','pppppp')' at line 1

I have also tried lots of different types of SQL injection and none of them work

No it isn't secure.... and just because you can't break it doesn't mean that somebody else can't — Mark Baker, Oct 16 '15 at 11:17
mysqli_query will only perform 1 query. Send it multiple queries as an input parameter and it will raise an error — Darren H, Oct 16 '15 at 11:17
'DROP table pdf ....... ;DROP table pdf .... DROP table pdf # .... 'DROP table pdf; .... 'DROP table pdf // .... on another form i've tried 'OR firstname LIKE %h% still nothing, literally tried all sorts — dev_py_uk, Oct 16 '15 at 11:18
SQL injection doesn't have to physically **break** something. It's sufficient that errors start popping up when your clients are browsing your website. Imagine that you have a shop and by a mistake you've passed a wrong parameter to a URL which causes your site to start spitting out warnings and MySQL errors. That's enough for a potential attacker to find out what your database structure is. The idea behind handling errors is that you can recover from them and not disclose your internal structure. — Mjh, Oct 16 '15 at 11:20
Most injections are not aimed to destroy you data... What about `whatever@mail.com' OR 1=1; --` in your SignUp?; — Julio Soares, Oct 16 '15 at 11:23
Anyway... `$forename` receives `'a","b","c","d","e","f"); DROP table admin; --'`, then you have: `"INSERT INTO admin (forename,surname,email,securityq, securitya,password) VALUES ("a","b","c","d","e","f"); DROP table admin;--','$surname','$email','$securityq','$securitya','$password')";` — al'ein, Oct 16 '15 at 11:24
I totally agree with Mark Baker and Mjh. Your code is not secure at all. Also use crypt function to store password. password should never be stored as it is. It should use a one way encryption technique. — Ashish Choudhary, Oct 16 '15 at 11:24
on my sign in page i've used field type ="email" which wont let you type any symbols after the @ and it forces you to put the @ symbol in — dev_py_uk, Oct 16 '15 at 11:25
@AshishChoudhary I've had many issues with my password encryption - check out my other question regarding this http://stackoverflow.com/questions/33098837/where-do-i-need-to-add-variables-in-hashed-password-script — dev_py_uk, Oct 16 '15 at 11:27
This code wont even reach till email field. As described by Alan Machado, it will break in forename only :) — Ashish Choudhary, Oct 16 '15 at 11:27
When in doubt, shorten what you need to a sentence of three or four words, type it in Google and add "best practices" at the end. Works for me. — al'ein, Oct 16 '15 at 11:28
this is my registration form - which - yes I agree with you it will break by forename, however although i should and protect this from SQL injection, it will only be used by a select few admin members one of which will be myself. the Sign In form has two fields - email - password, on these the SQL injection wont work — dev_py_uk, Oct 16 '15 at 11:29
@oggle0901 you should never rely on client side security. Maybe an up to date browser would ensure that there is something like an email address in your input type="email" but who would try to hack your page with a browser? It is absolutly reckless to think that with a type="email" attribute your signup is more secure than an input type="text". — hacksteak25, Oct 16 '15 at 11:31
As a programmer, we should have a habit of writing the best code we can. We shall NEVER assume that this cannot happen. — Ashish Choudhary, Oct 16 '15 at 11:31
I totally agree with what you're all saying, I was just getting off topic there — dev_py_uk, Oct 16 '15 at 11:32
While we are on this topic then and you've all made me well aware of the insecurities just because of my personal injection not working... would something like 'mysqli_real_escape_string' work? — dev_py_uk, Oct 16 '15 at 11:33
You are right. 'mysqli_real_escape_string' would work. Make a habit of using PDO instead of Procedural. Its easy, neat, well secured and is used in almost all the frameworks available. — Ashish Choudhary, Oct 16 '15 at 11:38
`mysqli_query` does only allow the execution of one single statement. For multiple statements, you need to use `mysqli_multi_query`. — Gumbo, Oct 16 '15 at 16:40

AbcAeffchen · Answer 1 · 2015-10-16T12:16:05.917

If I remember this right, mysqli_query can only execute one query at a time. This means, that you cannot expand the query via injection into more queries like

$query = "INSERT INTO admin (forename,surname,email,securityq, securitya,password) 
          VALUES ('$forename','$surname','$email','$securityq','$securitya','$password')";

using

$password = "'); DROP TABLE admins; --";

But maybe you can change it to overwrite some other data. E.g. using

$password = "newPasswort') ON DUPLICATE KEY UPDATE password = VALUES(password); --";

I've not tested this. It's just to get the Idea. DROP is not the only evil injection.

The best way yo prevent SQL injections is to use prepared statements. Escaping strings is better than do nothing, but maybe there are some examples where even the escaped string leads to an injection. Prepared statements are handed to the database separately, so that it knows the query already and inputs cannot change the query any more.

Also notice that storing the plain password is never a good idea. Please hash passwords using password_hash or something similar.

Thanks for your ideas on this, I'll do some more research and testing to see the results: - also I wonder if you could be of assistance on another of my questions which is still unresolved (regarding password hashing) - http://stackoverflow.com/questions/33098837/where-do-i-need-to-add-variables-in-hashed-password-script — dev_py_uk, Oct 16 '15 at 11:44

score 0 · Accepted Answer · answered Oct 16 '15 at 15:15

There are multiple ways to break your code; it is not secure. It doesn't have to be possible to execute a DROP statement for your code to be unsafe.

For example, the function SignUp is insecure. If the value of $_POST[email] is ' OR 1=1 --, then the authentication query becomes:

SELECT * FROM admin WHERE email = '' OR 1=1 --

(The -- is a common indicator of an attack; its purpose is to turn whatever follows into a comment.) This query will always return a result, because 1=1 is always true. So, your user will always be authenticated as an admin. This means newUser gets called, the attackers data gets inserted into the admin table, and you just lost control of your site.

MORAL: Always use prepared statements. Never directly insert a value that came from a user, could have come from a user, came from a database, came from an API, etc., directly into your SQL statements. Do not trust anybody (including yourself) when it comes to SQL injection, and use a prepared statement for every parameter or variable, every time.

You should read OWASP's guide to SQL injection issues, their SQL Injection Prevention Cheat Sheet, and their PHP Security Cheat Sheet.

I've tried SQL injecting my code which 'technically' should be vulnerable but it wont work

2 Answers2