Comparing $_POST values to database fields

Question

I'm pretty sure this code has something wrong with it:

$sql="select * from user where username={$_POST['username']}AND pwd=   {$_POST['password']}";

$r = mysqli_query($link,$sql);
if($r)
{ 
$_SESSION['loggedin']=true;

echo "Welcome". $_POST['username']; 
}

This is a very concise example of precisely what not to do. Three enormous hazards in five lines of code. — tadman, Apr 03 '13 at 19:16
@tadman I agree. There should be an ability to post the _worst ideas of all time_ on SO. — Colin M, Apr 03 '13 at 19:17
Welcome to Stack Overflow! This question is off topic and should be posted at [alt.se.coderev](http://codereview.stackexchane.com) — Cole Tobin, Apr 03 '13 at 19:18
The only thing missing here is an `eval` statement and the *four horsemen* will be present. — tadman, Apr 03 '13 at 19:18
@pd4627 The short answer is you're doing **everything** wrong. Please read Madra's answer. — tadman, Apr 03 '13 at 19:19
@ColeJohnson Yes, it would be - if it were a post. I just wish we could _promote_ (or, technically, demote) posts to that list. On another note, @tadman, you mean that `eval('md5("{$_POST[\'password\']}")');` isn't the right way to hash passwords!? Man, I have been doing it wrong. — Colin M, Apr 03 '13 at 19:20

score 5 · Accepted Answer · edited May 23 '17 at 11:45

5

Yes, you are exposed to SQL injection. Please learn about preapred statements.

Also, you apparently store passwords in plain-text. That's a security risk since if your database is exposed (due to SQL injection attack, for example, cough cough), all of your passwords could be compromised.

A few links:

edited May 23 '17 at 11:45

Community

1
1

answered Apr 03 '13 at 19:13

Madara's Ghost

172,118
50
264
308

The `mysqli` actually supports placeholders, so it should be easy to fix this part. – tadman Apr 03 '13 at 19:16
1

@tadman: The fact that it *supports* it helps. But you gotta use it, you know :) – Madara's Ghost Apr 03 '13 at 19:17
And no amount of driver support for prepared statements will save this person's database passwords or XSS issues :D. – Colin M Apr 03 '13 at 19:18
At least it does't require re-writing some horrible `mysql_query` based system. – tadman Apr 03 '13 at 19:18
Madara Uchiha is correct. Also you forgot to fetch the query. – Nik Drosakis Apr 03 '13 at 19:19

Comparing $_POST values to database fields

1 Answers1