Hacked issue

PHP
1

Hi,

I have a simple login script that apears to have been hacked, as someone managed to login and alter a recored with… Hacked By…

Here are snippets of code:

The code that logs user in:



case 'checkuser':

	foreach($_POST as $key=>$value){
		$$key = ValidateInput($value);
	}
	
	if ((!$username) || (!$password)) {
		echo '<br /><br /><center>Please enter ALL of the information!<p />Go back <a class="two" href="#" onClick="history.go(-1)">here</a></center>';
		return;
	}

	$npassword = md5($password);

	$sql = mysql_query("SELECT * FROM users WHERE username='$username' AND password='$npassword' AND activated='1'");
	$login_check = mysql_num_rows($sql);

	if ($login_check > 0) {
		while($row = mysql_fetch_array($sql)){
			foreach($row as $key=>$value){
				$$key = ValidateOutput($value);
	}

	session_register('userid');
	$_SESSION['userid'] = $userid;
	session_register('firstname');
	$_SESSION['firstname'] = $firstname;
	session_register('lastname');
	$_SESSION['lastname'] = $lastname;
	session_register('username');
	$_SESSION['username'] = username;
	session_register('email');
	$_SESSION['email'] = $emailaddress;
	session_register('special_user');
	$_SESSION['userlevel'] = $userlevel;
			
	echo '<center><b>Login Successfull</b><p />Please wait while you are redirected</center>';
	header("Refresh: 2; url=./index.php");
	
	}
		
	}else{
		echo '<center>You could not be logged in, Please retype your Username and Password<p />Go back <a class="two" href="#" onClick="history.go(-1)">here</a></center>';
	}

break;

On the top of each page has:



session_start();

Each case that is restricted to people logged in has following code:



case 'editrecord':

	if($_SESSION['userlevel'] != 3) {
	header('location: ./index.php?action=login');
	}

	// Secured code here

break;

The validate unput function to clean data before checking against database is:



function ValidateInput($value) {
	$value = strip_tags(trim($value)); 
	return $value; 
}

Where is the opening that will be allowing people to get in and make changes (hack the site)?

Cheers

2

You don’t know that the vulnerability was this code, or that the hacker logged in. The only way to access your database is NOT through your PHP script!

That said, your code is vulnerable to SQL injection. You never escape $username before inserting it into the query. What if someone entered this as their username:

admin'; --

Think about what that does to your query. It will select the row for the admin user while turning the rest of your conditions (checking the password and activation) into a comment to be ignored.

[color=red]SELECT * FROM users WHERE username='admin';[/color] --' AND password='$npassword' AND activated='1'

To fix the hole for now, use mysql_real_escape_string on the username. Long term, learn to use prepared statements and parameterized queries. Look at PDO and mysqli.

3

Oh yea good spotting… This is from an older site I did some time back…

The function I use now that validates data before any queries are performed is:



function ValidateInput($value) {
	$value = mysql_real_escape_string(strip_tags(trim($value))); 
	return $value; 
}

I also dont make use of session_register anymore…

Are there any tell tale signs of how people may have got in?

I tried the likes of admin’; – in the username and blank password as the code was and it didnt allow a successfull login.

I have had more recent sites tampered with that make use of mysql_real_escape_string… They only seem to edit a record once they gain access and dont tamper with the entire site…

Cheers

4

I really dislike functions like these. The way to sanitize input is not to throw all the functions you can think of at it. There’s no reason to ever call strip_tags on a username destined for a SQL query. There’s no reason to call mysql_real_escape_string on input that’s not going into a SQL query.

5

Yea I guess it is abit on the lazy side…

Given the code in the first post and the fact that mysql_real_escape_string has been added… Can they get passed this:



case 'editrecord':      

if($_SESSION['userlevel'] != 3) {     
  header('location: ./index.php?action=login');
}

  // Secured code here  break;  

break;

and edit records using the code below the userlevel check within the case?

Can that and the login be ruled out and assume they are getting access in another manner?

Cheers