Can this php code be sql injected?

Question

I'm trying to learn SQL injection to become a white-hat but I find it quite difficult ...

I found this code on a site and it's told to be a easy-difficulty.

The HTML code has a username ,password boxes and a button.

if(isset($_POST['autentificare']) && $_POST['autentificare']=='OK' && $_POST['admin']!="" && $_POST['adm_password']!="")
{
    $admin=$_POST['admin'];
    $adm_password=$_POST['adm_password'];
    $login="SELECT admin,password FROM owner where admin='".$admin."' AND password='".$adm_password."' ";
    $result_auth=mysql_query($login,$db) or die("Query failed: ".mysql_error()." Actual query: ".$login);
    $user_identity;
    while($dates = mysql_fetch_object($result_auth))
    {
        $user_identity=$dates->admin;
        $password_ident=$dates->password;
    }
    if($result_auth && $user_identity==$admin && $password_ident==$adm_password)
    {
        $_SESSION['adm_username']=$admin;
        $_SESSION['adm_password']=$adm_password;
        $authval="V";
    }
    else
    {
        $authval="D";

    }


}

Yes. Prepared/parameterized queries are not used, and the value of `$_POST['admin']` isn't verified. — Brad, Nov 13 '14 at 19:18
What exactly is SQL injection? → [The Great Escapism (Or: What You Need To Know To Work With Text Within Text)](http://kunststube.net/escapism/) — deceze, Nov 13 '14 at 19:18
I would like to know some methods to do it . I know it can be injected since it's a test code which is supposed to test me. — Andrew V, Nov 13 '14 at 19:20
Yes it can. If I load the page and add to the querystring something like admin=0; it will take it and run it. Password is probably better since its at the end of your sql statement. — Marshall Tigerus, Nov 13 '14 at 19:20
I learned something off sites where you would do "' in the username box but I don't really know what to do after — Andrew V, Nov 13 '14 at 19:21
In this case, an attack vector would be to use a value like `' UNION SELECT 'admin', 'password' -- ` with static values `UNION`'d to the intended query, which would cause the query to return `'admin'` and `'password'` as literals. Since `password` was also posted as the password, hooray, you're now an admin. — Michael Berkowski, Nov 13 '14 at 19:22
You are not doing *any* sort of escaping/security here, so of course it can be be injected. It's trivial to hack this code. Send `' OR 1=1; -- ` as `admin`, done. `password` is ignored since `-- ` starts a comment in MySQL. — gen_Eric, Nov 13 '14 at 19:25
@RocketHazmat But there's still a check for a matching password in code, so the value of `$_POST['adm_password']` also needs to be returned from the query. — Michael Berkowski, Nov 13 '14 at 19:26
@MichaelBerkowski: Ah! You're right. Didn't see that. That would explain why you did `UNION` in your attack. :-) — gen_Eric, Nov 13 '14 at 19:28
@MichaelBerkowski can you tell me the exact syntax I would have to use ? and maybe some documentation where I could learn more? — Andrew V, Nov 13 '14 at 19:29
@AndrewV The more I think about it, the more trouble I'm having defeating the fact that _both_ the input username and password are compared against the values returned by the SQL. I can't answer unless I fully solve it. What I was going for was a UNION attack https://www.owasp.org/index.php/Testing_for_SQL_Injection_(OTG-INPVAL-005)#Union_Exploitation_Technique — Michael Berkowski, Nov 13 '14 at 19:44
I read about Union Exploitation Technique from the site you linked , and I understood somehow , I'm thinking about a way to use it myself. — Andrew V, Nov 13 '14 at 19:54

score 2 · Answer 1 · answered Nov 13 '14 at 19:26

2

Your best "weapon" against SQL injection is prepared statements. With this way you do not mix code with data...as you do in your queries above.

So of course...you are vulnerable to SQL injection.

By mixing code and data the attacker can send the input in such a way that in effect can alter the form of the query.

answered Nov 13 '14 at 19:26

Dimitris Papageorgiou

417
4
17

Prepared statements can be fooled as well [Some reading material](http://stackoverflow.com/questions/134099/are-pdo-prepared-statements-sufficient-to-prevent-sql-injection) – DarkBee Nov 13 '14 at 19:28
@DarkBee: Prepared statements are safe. The attack in that answer is because the programmer isn't using a prepared statement later on in the code. If you use them *everywhere* then you are fine. The attack in that answer was because of string concatenation/non-prepared statements. – gen_Eric Nov 13 '14 at 19:31

score 1 · Answer 2 · answered Nov 13 '14 at 19:21

1

Yes. Your query is SQLi vulnerable. I strongly suggest using PDO. It takes care of escaping queries for you. http://wiki.hashphp.org/PDO_Tutorial_for_MySQL_Developers#Named_Placeholders

answered Nov 13 '14 at 19:21

Mike

842
1
9
31

score 0 · Answer 3 · answered Nov 13 '14 at 19:28

Obvious problems with the code from a security/sql injection standpoint:

Data isn't cleansed. Anytime you take inputs from a user and process it in a query you want to cleanse it by encoding any special characters. There are a variety of functions that can do this, and since you're a student I'll let you research that bit (learning how to find functions for your purposes is a vital skill in development).
You are not using bound parameters (and you're not using $mysqli which you should be). Bound parameters help build the query in a way that the application knows what sort of inputs to expect. It stops people from sneaking a subquery into your code.
Passwords are being stored in an unencrypted state. Passwords should always be encrypted, so if they are stolen its not as bad as if they were unencrypted.

That should get you started.

Can this php code be sql injected?

3 Answers3