runnerjp Posted April 25, 2007 Share Posted April 25, 2007 hey after resent possible sql injection ( i think) seems my login script needs tightening up also i would like to know what they did to get into my site lol sum times i think that knowing how they did is best way to stop it! <?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; $login = preg_replace("/[^a-zA-Z0-9]/", "", $login); //this is what just got added, would this do? $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"; } } ?> Quote Link to comment Share on other sites More sharing options...
kalivos Posted April 25, 2007 Share Posted April 25, 2007 That will restrict all users to not having any special characters in their login name. Another approach would be to clean the input this way... <?php function read_member ($login) { global $db_name, $tbl_members; $result=mysql_fetch_array(mysql_db_query($db_name, "SELECT * FROM ".add_slashes($tbl_members)." WHERE login = '".add_slashes($login)."'")); return $result; } function authenticate ($login, $password) { global $db_name, $tbl_members; $login = add_slashes($login); $tbl_members = add_slashes($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"; } } ?> Quote Link to comment Share on other sites More sharing options...
sw0o0sh Posted April 25, 2007 Share Posted April 25, 2007 I heard of enclosing sql areas with {} mysql_query("SELECT * FROM users WHERE username = \"{". (htmlspecialchars(addslashes($_POST['username']))) ."}\" AND password = \"{".(md5($_POST['password']))."}\""); Thats what I do, or somewhat like it. Do things how you'd like. Quote Link to comment Share on other sites More sharing options...
MadTechie Posted April 25, 2007 Share Posted April 25, 2007 add_slashes() doesn't stop SQL injection all you need is the multi-byte character for a single quote of that character set used in the database mysql_real_escape_string() (mysql_escape_string() for PHP versions before 4.3.0) is better but this was not always safe.. Security fix: An SQL-injection security hole has been found in multi-byte encoding processing. The bug was in the server, incorrectly parsing the string escaped with the mysql_real_escape_string() C API function. (CVE-2006-2753, Bug#8378) Discussion: An SQL-injection security hole has been found in multi-byte encoding processing. An SQL-injection security hole can include a situation whereby when a user supplied data to be inserted into a database, the user might inject SQL statements into the data that the server will execute. With regards to this vulnerability, when character set unaware-escaping is used (for example, addslashes() in PHP), it is possible to bypass the escaping in some multi-byte character sets (for example, SJIS, BIG5 and GBK). As a result, a function such as addslashes() is not able to prevent SQL-injection attacks. It is impossible to fix this on the server side. The best solution is for applications to use character set-aware escaping offered by a function such mysql_real_escape_string(). However, a bug was detected in how the MySQL server parses the output of mysql_real_escape_string(). As a result, even when the character set-aware function mysql_real_escape_string() was used, SQL injection was possible. This bug has been fixed. Link Quote Link to comment Share on other sites More sharing options...
runnerjp Posted April 25, 2007 Author Share Posted April 25, 2007 ok was not sql injection.... after cheaking it out the problem is that i use member ids that show up when clicked view source on browser... all 1 has to do then is save the script change the member number then whala you can change any 1ns password .... any 1 have any ideas how to solve this?? <dir inc="accountinfo"> <? if($action == "update") { if($auth['login'] == "admin" AND $demo_mode == "yes") { include("include/admin_demo.inc.php"); exit; } //check input for errors $pass_length = strlen("$pass1"); if(empty($pass1)) { echo "You did not enter a password! Please <a href=\"javascript:history.go(-1);\">Try again.</a>"; include("include/footer.inc.php"); exit; } elseif(empty($pass2)) { echo "You did not verify your password! Please <a href=\"javascript:history.go(-1);\">Try again.</a>"; include("include/footer.inc.php"); exit; } elseif("$pass1" != "$pass2") { echo "Your passwords do not match! Please <a href=\"javascript:history.go(-1);\">Try again.</a>"; include("include/footer.inc.php"); exit; } elseif(empty($email)) { echo "You did not enter your email! Please <a href=\"javascript:history.go(-1);\">Try again.</a>"; include("include/footer.inc.php"); exit; } elseif(empty($displayname)) { echo "You did not enter your Display Name! Please <a href=\"javascript:history.go(-1);\">Try again.</a>"; include("include/footer.inc.php"); exit; } elseif($pass_length < 3) { echo "Your password must be at least 3 characters long. Please <a href=\"javascript:history.go(-1);\">Try again.</a>"; include("include/footer.inc.php"); exit; } elseif(ereg("^[_a-z0-9-]+(\.[_a-z0-9-]+)*@([a-zA-Z0-9-]+\.)+([a-zA-Z]{2,3})$", $email)) { $okmail="1"; } if($okmail != "1") { echo "Your email address is not properly formatted! Please <a href=\"javascript:history.go(-1);\">Try again.</a>"; include("include/footer.inc.php"); exit; } if (ereg("^[a-zA-Z0-9]+$",$pass1)) { $okpass="1"; } if($okpass != "1") { echo "Your password can contain only letters and numbers! Please <a href=\"javascript:history.go(-1);\">Try again.</a>"; include("include/footer.inc.php"); exit; } if (ereg("^[a-zA-Z0-9]+$",$displayname)) { $okdisplay="1"; } if($okdisplay != "1") { echo "Your Display Name can contain only letters and numbers! Please <a href=\"javascript:history.go(-1);\">Try again.</a>"; include("include/footer.inc.php"); exit; } if(empty($newsletter)) $newsletter = "no"; $connection = @mysql_connect("$db_host", "$db_user", "$db_pass") or die("Couldn't connect."); $db = @mysql_select_db($db_name, $connection) or die("Couldn't select database."); $sql = "UPDATE $tbl_members SET password = \"$pass1\", email = \"$email\", displayname = \"$displayname\", newsletter = \"$newsletter\" WHERE member_id =\"$member_id\""; $result = @mysql_query($sql,$connection) or die("Couldn't execute update query."); echo "<center>Update successful!<br><br>If you changed your password, you <b>must</b> <a href=\"logout.php\">Login again!</a></center>"; } else { if($auth['login'] == "admin" AND $demo_mode == "yes") { include("include/admin_demo.inc.php"); exit; } ?> <form action="<?echo "$PHP_SELF"; ?>" method="POST"> <table align="center" cellpadding="4" cellspacing="0"> <tr> <td valign="top"> <p>Username:</p> </td> <td valign="top"> <p><b> <?echo $auth['login'] ?></b></p> </td> <td valign="top"> <p><span class="help">Your Username cannot be changed.</span></p> </td> </tr> <tr> <td valign="top"> <p>Password:</p> </td> <td valign="top"> <p><input type="password" name="pass1" maxlength="20" size="20" value="<?echo $auth['password'] ?>"></p> </td> <td valign="top"> <p><span class="help">Your Password may be 3-20 characters, letters and or numbers only.</span></p> </td> </tr> <tr> <td valign="top"> <p>Repeat Password:</p> </td> <td valign="top"> <p><input type="password" name="pass2" maxlength="20" size="20" value="<?echo $auth['password'] ?>"></p> </td> <td valign="top"> <p><span class="help">Please verify your password.</span></p> </td> </tr> <tr> <td valign="top"> <p>Email Address:</p> </td> <td valign="top"> <p><input type="text" name="email" maxlength="125" size="25" value="<?echo $auth['email'] ?>"></p> </td> <td valign="top"> <p><span class="help">Your email address is used to retrieve lost password. It is not displayed to the public.</span></p> </td> </tr> <tr> <td valign="top"> <p>Display Name:</p> </td> <td valign="top"> <p><input type="text" name="displayname" maxlength="25" size="25" value="<?echo $auth['displayname'] ?>"></p> </td> <td valign="top"> <p><span class="help">This is your nickname or the name you want the system to refer to you by.</span></p> </td> </tr> <tr> <td valign="top"> <p> </p> </td> <td width="757" valign="top" colspan="2"> <p><input type="checkbox" name="newsletter" value="yes" checked>Subscribe to Newsletter updates (occasional updates regarding the site).</p> </td> </tr> <tr> <td valign="top"> <p><input type="hidden" name="action" value="update"> <input type="hidden" name="member_id" value="<?echo $auth['member_id'] ?>"></p> </td> <td valign="top"> <p><input type="submit" value="Update!"></p> </td> <td valign="top"> <p> </p> </td> </tr> </table> </form> <p> </p> <? } ?> </dir> see <?echo $auth['member_id'] ?> thats where it changes the information so any 1 can chnage any 1ns password! Quote Link to comment Share on other sites More sharing options...
clown[NOR] Posted April 25, 2007 Share Posted April 25, 2007 i always use a random session id. on login I give the person a random number between 1000 and 10000. And it's uniqe for each user. so they cannot change any info on other rows than where the session id is the same as their id and username. havent had any problems with that yet. so it's 1 of 2.. 1) it's safe and they havent been able to hack it.... or .... 2) nobody has tried to hack it Quote Link to comment Share on other sites More sharing options...
runnerjp Posted April 26, 2007 Author Share Posted April 26, 2007 haha true but surly this is easy 2 do...all users will have to do is go on a profile... the profile will have the id on it...shazam they can change it... is there away to hide the hidden information <input type="hidden" name="action" value="update"> <input type="hidden" name="member_id" value="<?echo $auth['member_id'] ?>"> so its actually hidden! even in view source ?? Quote Link to comment Share on other sites More sharing options...
MadTechie Posted April 26, 2007 Share Posted April 26, 2007 ooow hidden fields are useless, the only reason to use them is for making things "LOOK" tidy i suggect having an extra field on the form.. called old password, then have the system check the old password field with the current password (same as login) before doing anything else, just a quick fix but i am swamped Quote Link to comment Share on other sites More sharing options...
clown[NOR] Posted April 26, 2007 Share Posted April 26, 2007 also a good idea MadTechie... but that's not how I check the sessions ID runnerjp... what I've done is that I'm storing username and sessionID in cookies and i run it trough a function chkLogin() before I let the person do any changes. this is my chkLogin() function: <?php function chkLogin() { if (isset($_COOKIE['sessionID'])) { $cSessionID = $_COOKIE['sessionID']; $cStatus = $_COOKIE['status']; $cUserName = $_COOKIE['username']; #echo "Session ID: ".$cSessionID."<br>"; #echo "Status: ".$cStatus."<br>"; #echo "Username: ".$cUserName."<br>"; if (empty($cSessionID) || empty($cStatus) || empty($cUserName)) { return false; } else { c2db("users"); $cSessionID = mysql_real_escape_string($cSessionID); $cStatus = mysql_real_escape_string($cStatus); $cUserName = mysql_real_escape_string($cUserName); $query = "SELECT * FROM users WHERE bruker = '".$cUserName."' AND status = '".$cStatus."' AND sessionID = '".$cSessionID."'"; $result = mysql_query($query); if (!$result) { die("Could not run query from database"); } $dbRows = mysql_num_rows($result); if ($dbRows > 0) { return true; } elseif ($dbRows == 0) { return false; } } } else { return false; } } ?> Quote Link to comment Share on other sites More sharing options...
runnerjp Posted April 26, 2007 Author Share Posted April 26, 2007 would is completely prevent it? Quote Link to comment Share on other sites More sharing options...
runnerjp Posted April 26, 2007 Author Share Posted April 26, 2007 would this work 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";} } and if so where would i put it in <?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; $login = preg_replace("/[^a-zA-Z0-9]/", "", $login); //this is what just got added, would this do? $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"; } } ?> Quote Link to comment Share on other sites More sharing options...
clown[NOR] Posted April 26, 2007 Share Posted April 26, 2007 well, I have the chkLogin() inside functions.php, and then when I'm making a page that is restricted to members only I start with <?php $chkLogin = chkLogin(); if (isset($chkLogin)) { echo "you're logged in"; } else { echo "you dont have access to this page"; } ?> Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.