-3

This question already has an answer here:

Please note: this isn't a duplicate!!! Why? You can use ' but You don't know how :)

I have this PHP code:

<?php
(...)

function escape($str) {
$ret = '';
for($i=0;@$str[$i];$i++)
{
    if($str[$i]!='\'')
    if($str[$i]!='\\')
    if($str[$i]!='"')
    if($str[$i]!="\r")
    if($str[$i]!="\n")
    if($str[$i]!="\x1a")
    {
        $ret .= $str[$i];
        continue;
    }
    $ret .= '?';
}
return $ret;
}

$pass = escape($_POST['password']);

$query = "select 1 from user_pwd where pass='{$pass}'";
$query = mysql_query($query);
$array = mysql_fetch_array($query);
var_dump($array);
?>

I know that's isn't safe (I know SQL Injection, I can do mysql_real_escape_string or add_slashes, but I just want to know what's wrong :).

EDIT: You can use '. If You send array in POST data: password[0]=1&password[1]=' union select '1 You will do SQL Injection ^^

marked as duplicate by tim, Community Aug 4 '15 at 17:20

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

  • 4
    Please consider posting on code review also, you will have a lot to learn about code style. – Cthulhu Aug 4 '15 at 14:39
  • 4
    The mysql extension is deprecated (you should NOT be using it). Use mysqli or PDO. I would go with PDO. – Sakamaki Izayoi Aug 4 '15 at 16:20
  • 1
    What if my password is 0mysuperawesomepassword? Suddenly, my password isn't secure at all. – tim Aug 4 '15 at 17:15
2

First, as you already observed, it's open to SQL injection. SQL injection isn't limited to things that start with "--" or "//"; it's only limited by the creativity of the attacker.
Use parameterized queries. Always.

Second, you should hash the password. PHP's builtin functions provide you with systems that automatically hash and provide a salt, so you have no excuse.

The purpose of filtering is often to prevent XSS attacks, but these happen when you show data that was entered by one user (the attacker) to another user. I sure hope you're not planning on showing people's passwords to random visitors...

  • But how someone can get my passwords? He can "only" comment the code. – Szymon Marczak Aug 4 '15 at 14:16
  • It's only limited by the creativity of the attacker. But in this specific case your filter doesn't seem to filter on ' ... so the attacker could turn your statement into where pass=''; select * from user_pwd – S.L. Barth Aug 4 '15 at 14:19
  • 8
    Just use parametrized queries... No need for all kinds of weird functions... – Arperum Aug 4 '15 at 14:23
  • 2
    You are also lowering the entropy. If the password is qwerty' and you enter qwerty", it will be accepted after your substitution. (Yes that's a bad password I know ;)) – r00t Aug 4 '15 at 14:31
  • 4
    The entropy issue is even worse: the loop condition is @$str[$i] but PHP casts zero ('0') to FALSE in a boolean context and this will terminate the loop when it encounters a zero. So a password like 'F00bar6294!$()' would be considered equal to 'F'. – John Morahan Aug 4 '15 at 15:09

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