login script needed looking at :)

[runnerjp]

hey guys... i seem to have had a "bug" within my login code because some 1 has been able to access my admin account and change the password!

could some one check it all over and see where i may have gone wrong ty

 

<?

// File ID: login.php (user log in routine)

include("require/config.php");
require("require/membership.php");

$content="include/loginbox.inc.php";
$menu="include/menu_u.inc.php";
$page_title="Login Form";

if ($HTTP_POST_VARS) {
    if ($login && $password) {
$password=crypt($password, $login);
$data=authenticate($login, $password);
if ($data[error]) {$error=$data[error];}
else {
	setcookie("ProfilePHP","$login&&$password", 0, "/");
	if (!$ref) {$ref="index.php";}
	Header("Location: redirect.php?ref=$ref");
}
  } else {$error="901";}
}

error_message($error);
?>

 

here is login table

<table width=335 cellpadding=10 cellspacing=1 border=0 bgcolor=#B9DCFF>
  <form action="login.php" method=post><tr><td width="313" height="125" bgcolor="#F9FBFD"><table width=311 cellpadding=0 cellspacing=0 border=0>
    <tr>
      <td colspan=3 height=30 valign=top><div align="center"><font face=arial size=2 color=#0e3f5d style="font-size:18px;letter-spacing:-1px;">Existing members log in here:</font></div></td>
      <td width="45" align=right valign=top><div align="center"><font face=Arial size=1 class=bl><a href="forgot.php"><span> Lost Passwords</span></a></font></div></td>
      <td align=right valign=top><a href="forgot.php"><img src="images/questionmark.jpg"  border=0 align="left" /></a></td>
    </tr>
    <tr>
      <td width="53" height=15><div align="center"><font color=#666666 face=Arial size=1 style="font-size:11px;">Member name</font>:</div></td>
      <td width="180"><div align="left">
        <input type="text" name="login" maxlength="30" size="30" style="font-family: Georgia, "times new roman", times, serif; font-size:9pt;">
      </div></td>
    </tr>
    <tr>
      <td width="53" height=15><div align="center"><font color=#666666 face=Arial size=1 style="font-size:11px;">Password:</font></div></td>
      <td><div align="left">
        <input type="password" name="password" maxlength="20" size="29" style="Georgia, "times new roman", times, serif; font-size:9pt;">
      </div></td>
    </tr>
    <tr>
      <td height=5 colspan=5><img alt="" height=5 width=1 /></td>
    </tr>
    <tr>
      <td bgcolor=#e0e3eb height=1 colspan=5><img alt="" height=1 width=1 /></td>
    </tr>
    <tr>
      <td colspan=4 height=32><div align="left"><font color=#666666 face=Arial size=1 style="font-size:11px;" class=bl><a href="register.php">Join now!</a> Get your running profile in seconds! </font>
        <input border=0  name=hp_log_in src="images/loginbutton.jpg" type=image />
      </div></td>
      <td width="32" align=right valign=bottom> </td>
    </tr>
  </table></td>
  </tr> </form></table>

 

here is membership script

 

<?php

function read_member ($login) {
global $db_name, $tbl_members;
$result=mysql_fetch_array(mysql_db_query($db_name, "SELECT * FROM $tbl_members WHERE login = '$login'"));
return $result;
}

function authenticate ($login, $password) {
global $db_name, $tbl_members;
$valid = mysql_fetch_array(mysql_db_query($db_name, "SELECT * FROM $tbl_members WHERE login='$login'"));
if ($login) {
	if ($password ==  crypt($valid[password], $login)) {
		if ($valid[enabled] ==  "yes") {$result=$valid;} else {$result[error]="700";}
	} else {$result[error]="800";}
} else {$result[error]="200";}
return $result;
}

function error_message ($error) {
global $incpath;
if ($error) {
	include("$incpath/error.inc.php");
	$GLOBALS["content"]		=${"strError$error"};
	$GLOBALS["page_title"]	="Error: #$error";
}
}

?>

 

any ideas?

or was it just brute force :S

[runnerjp]

"bump"

[MadTechie]

$login isn't filtered

 

<?php

$login = preg_replace("/[^a-zA-Z0-9]/", "", $login); //limits username to numbers and letters

?>

 

EDIT: in the function authenticate

[runnerjp]

where abouts do i add this sorry??

[MadTechie]

here is membership script

 

<?php
function authenticate ($login, $password) {
global $db_name, $tbl_members;
$login = preg_replace("/[^a-zA-Z0-9]/", "", $login); //limits username to numbers and letters
$valid = mysql_fetch_array(mysql_db_query($db_name, "SELECT * FROM $tbl_members WHERE login='$login'"));
if ($login) {
	if ($password ==  crypt($valid[password], $login)) {
		if ($valid[enabled] ==  "yes") {$result=$valid;} else {$result[error]="700";}
	} else {$result[error]="800";}
} else {$result[error]="200";}
return $result;
}
?>

 

as a side note this was on just a quick overview their maybe more bugs (but i am at work)

[bobleny]

I don't have my script with me or I'd show you, but what you need to do is control what your user puts in text fields.

 

If you don't control your users, you are highly susceptible to MySQL injection.

http://en.wikipedia.org/wiki/SQL_injection

 

Using preg_match() will allow you to control your users...

http://us2.php.net/preg_match

 

Basically,

if (preg_match("/[^a-zA-Z0-9_-]/", $string))

{

  //Send your user back to do it again.

  //What ever they typed in contains invalid characters

  //The may be trying to attack you!

}

else

{

  //They don't appear to be hacking you...

}

 

Using various work arounds, you may be able to manipulate your code enough to make it even harder to be hacked.

 

I wish I had my sign up sign in script... I don't think it is hackable! But it is much longer as a result!

[runnerjp]

thnaks guys... been reading up in mysql injection stuff but i dont get how to change to code to stop it :S

[MadTechie]

basically if you filter the characters that are used to inject the code ie ' and " then your safe

hence the filter

$login = preg_replace("/[^a-zA-Z0-9]/", "", $login); //limits username to numbers and letters

[runnerjp]

ahh right... why how have people being useing " and ' to get into my account?

[MadTechie]

try these login details

username =

administrator' or 1=1

password =

blarblarblar

 

 

(if you havn't added my filter)

[runnerjp]

i have added it and seems to work

[MadTechie]

as the password is encrypted its possible but less likely that will be used for injection (would be hard to create the injection) MD5 works better,

 

basically you would have

 

SELECT * FROM $tbl_members WHERE login='$login' and password ='md5($password)'

 

also do this for insert (when inserting the password)

 

you may want to leave that for now