Jump to content

SQL injection


fontaine1

Recommended Posts

Using PHP/MySQL for an admin login form.

 

It is session-based, checking username / password against an admin table. Apparently, it is open to SQL injection, but I have no idea why. I was under the impression that either mysql_real_escape_string() or addslashes() would prevent injections to a login form. Apparently I am wrong.

 

Can anyone provide me with examples of how an injection could occur using the following sample query:

 

$sql = "SELECT * FROM members WHERE Email='$email' AND Password='$pass' AND Validated='1' AND Status='1'";
$result = @mysql_query($sql);  

 

Any tips on preventing this? Thankfully, this is a pretty innocuous site, not a lot of damage can be done, but I want to ensure that I have a solid grasp on what is occurring and how to prevent it.

 

Thanks!

Link to comment
Share on other sites

First, STOP using the mysql_ functions. They are no longer supported in current versions of PHP. Instead use the mysqli_ functions or, better yet, PDO for database operations.

 

Second, learn how to use prepared statements instead of trying to manipulate the data before running the query using things such as mysql_real_escape_string(). Prepared statements will prevent SQL Injection if done properly.

Link to comment
Share on other sites

First off, who says that your application has SQL injection vulnerabilities? A professional penetration tester? An automated scanner? Yourself?

 

Secondly, that two lines of code don't tell us anything, because we can't see what you're doing with the input. You claim that you're escaping it, but this critical part is exactly what you've left out.

 

The mysql_* functions in generally are hopelessly outdated (since more than a decade) and have been removed from PHP. Nowadays, we use PDO and prepared statements.

Link to comment
Share on other sites

Besides that, your code has plenty of other problems. You're either storing the passwords as plaintext, or you're using some toy password hash scheme like md5(). This is unacceptable. In fact, if there actually is an SQLi vulnerability, it's safe to assume that all current passwords are compromised.

 

And why on earth would you use the error suppression operator “@”? Errors need to be logged so that you can fix them, not suppressed. If your PHP setup prints error messages directly on the screen, that's yet another problem.

Link to comment
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.