PHP CMS Login Hack

Question

Someone recently hacked my PHP CMS and planted an SQL injection. Is there a way to make my login code more protected and prevent hackers? Any help would be great, thanks.

Login Form

<div id="loginform">

  <form method="post" action="check-login.php" name="form1">

    <label for="username" /><span style="color:#FFFFFF; font-family:'Trebuchet MS', Arial, Helvetica, sans-serif;">username:</span></label>

    <input type="text" name="myusername" id="username"/>

    <label for="password"/><span style="color:#FFFFFF; font-family:'Trebuchet MS', Arial, Helvetica, sans-serif;">password:</span></label>

    <input type="password" name="mypassword" id="password"/>

    <label for="submit"></label>

    <input type="submit" name="sumbit" value="Login">

  </form>

</div>

PHP

mysql_connect ($host, $username, $password) or die ("can't connect");
mysql_select_db ($db_name) or die (mysql_error());

$myusername = $_POST['myusername'];
$mypassword = $_POST['mypassword'];

$sql = "SELECT * FROM $tbl_name WHERE username='$myusername' and password='$mypassword'";
$result = mysql_query($sql);

$count = mysql_num_rows($result);
if ($count == 1){
 session_register("myusername");
 session_register("mypassword");
 header("Location:cms/admin.php");
}else{
 echo "Wrong username or password";
}

What makes you think it was this script that was the problem? What CMS are you using? — Paul Dessert, Feb 17 '12 at 18:38
Read about http://www.php.net/real_mysql_escape_string and apply it here `$myusername = mysql_real_escape_string($_POST['myusername']);` and the same for the next line of the code. — Cheery, Feb 17 '12 at 18:39
I suspect given that this small bit of code isn't protected against basic SQL injection exploits the rest of your code is similarly insecure. You need a whole application rewrite, most likely, or just use an existing, tested CMS like Drupal and benefit from a halfway decent pre-built infrastructure. — ceejayoz, Feb 17 '12 at 18:44
**Upgrade yourself to PDO.** If you use [`PDO`](http://php.net/manual/en/book.pdo.php) with its built-in prepared statement functionality you don't have to worry about manual escaping. The `mysql*` are dinosaurs that shouldn't be used. — , Feb 17 '12 at 18:45
To further explain why your script is vulnerable, imagine what your SQL query would look like if someone put the following string into the password box: `' OR username = 'admin`. The query would always return a result for the admin, ignoring the password. This allows users to login as a privileged user and do whatever they like. — Tim Fountain, Feb 17 '12 at 18:46
Does this answer your question? [How can I prevent SQL injection in PHP?](https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) — Dharman, Sep 04 '20 at 20:33

HellaMad · Accepted Answer · 2012-02-17T18:47:43.877

6

Wow. This is a big no-no:

$myusername = $_POST['myusername'];
$mypassword = $_POST['mypassword'];

You need to sanitize these inputs with at least mysql_real_escape_string.

Change it to this:

$myusername = mysql_real_escape_string($_POST['myusername']);
$mypassword = mysql_real_escape_string($_POST['mypassword']);

htmlentities() and htmlspecialchars() are also useful but I would advise against using them on data going into a database.

edited Feb 17 '12 at 18:47

answered Feb 17 '12 at 18:40

HellaMad

5,294
6
31
53

Do you still need to sanitize the inputs in a `select` statement? I personally do, and I'm not doubting your answer, I just thought it only mattered in `insert` statements – Paul Dessert Feb 17 '12 at 18:46
1

@Paul **YOU ABSOLUTELY NEED TO SANITIZE** inputs in select queries. Let me introduce you to my friend [Bobby Tables](http://bobby-tables.com/) ... – Feb 17 '12 at 18:49
@Paul Yes! Any SQL query needs to be sanitized. Read up on SQL injections and you'll understand. Usually a hacker will insert a mySQL comment (`-- `) which then allows them to do _whatever they want_. – HellaMad Feb 17 '12 at 18:50
1

@Paul In general, you should do more than just escape the input. You should also filter input. In the case of a username:password, you should remove all characters that are not accepted. User names that are emails should always be evaluated to be a proper email format. Users names that are only "A-Z", "a-z", "0-9" and maybe dashed and underscores "-_" should be have all other characters stripped. Also, use something like HTMLPurify for large text input that could use HTML and familiarize yourself with XSS (cross-site scripting) – jmbertucci Feb 17 '12 at 19:14

score 2 · Answer 2 · answered Feb 17 '12 at 18:40

The problem with your code is that you directly place the user input from the login boxes into the SQL query. This indeed is a security hole and allows SQL injection.

You can use the mysql_real_escape_string() function to get around this - just pass the variables from the login form through it before putting it into the SQL query.

However, my preferred solution is to use PDO or MySQLi prepared statements - and I would strongly encourage you to learn about and use this method.

score 1 · Answer 3 · edited Sep 04 '20 at 20:34

1

YES

$myusername = $_POST['myusername'];
$mypassword = $_POST['mypassword'];

should be protected

$myusername = mysql_real_escape_string ($_POST['myusername']);
$mypassword = mysql_real_escape_string ($_POST['mypassword']);

mysql_real_escape_string()

edited Sep 04 '20 at 20:34

Dharman

30,962
25
85
135

answered Feb 17 '12 at 18:39

Charles Forest

1,035
1
12
30

score 1 · Answer 4 · edited May 23 '17 at 12:15

1

Short answer: yes. Your login code is wide open to SQL injection. You need to look into escaping user input. Take a few hours to read about SQL injection, understand the concept and explore the different solutions available. Using PDO and its prepared statements is a good way to deal with input escaping in PHP. This SO question and its top answers is a nice resource to start reading.

edited May 23 '17 at 12:15

Community

1
1

answered Feb 17 '12 at 18:44

Viktor

3,436
1
33
42

SachinGutte · Answer 5 · 2012-02-17T19:00:57.417

Login systems are always too difficult to implement though it seems to be very easy just by checking username and password for a match with existing record. There are a lot of ways to implement it.

Use salt and pepper applied on password submitted by user when he/she registers.
Check password for match using same hash algorithm you've used while registering.
Change user passowrd's hash value each time he successfully logs in.
If you are using sessions and cookies, there are many ways to secure them.
Cleaning all type of user inputs is very important.
Do not store usernames and passwords in sessions or cookies. (very bad)
Use captcha's to prevent any type of bot attacks.

All in one, you've to research about lot of things. If the CMS you re using doesn't use such mechanism, redesign it and then use. If this is not possible or feasible, the use most stable CMSs like Wordpress, Joomla or my favorite Drupal !

PHP CMS Login Hack

5 Answers5