-3

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 ^^

1 Answers1

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 04 '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 is on codidact.com Aug 04 '15 at 14:19
  • If and when an attacker compromises your database, he'll gain full access to the users table. If you store cleartext passwords, he can immediately steal user credentials and use them on your site / other sites, if the passwords were reused. Salting and hashing passwords makes it much more difficult for the an attacker to actually access the credentials. – etherealflux Aug 04 '15 at 14:20
  • But function "escape" will replace ' and " to "?"... – Szymon Marczak Aug 04 '15 at 14:22
  • 8
    Just use parametrized queries... No need for all kinds of weird functions... – Arperum Aug 04 '15 at 14:23
  • Ah, wait, my bad - you do filter out ' . But, once again: it only depends on the creativity of the attacker. A clever attacker WILL find a way. – S.L. Barth is on codidact.com Aug 04 '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 04 '15 at 14:31
  • @SzymonMarczak Questions on Stack Exchange remain open, so that in the future others may add new answers. When you marked my answer as "accepted" you gave a signal that for you, the issue is solved. But that still allows others to post new answers. (You can also un-accept my answer; it won't change the status of your question. It merely signals that for you, the issue hasn't been solved yet). – S.L. Barth is on codidact.com Aug 04 '15 at 14:38
  • 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 04 '15 at 15:09