Potatis Posted April 23, 2009 Share Posted April 23, 2009 I have been using a login script, which works, but not when I attempt to sanitize it. I don't know why. I have tried some functions from searches here, but nothing works so far. Here is the the code with the gaping wide sql injection hole: <?php session_start(); if($_SERVER['REQUEST_METHOD'] == "POST") { $username = $_POST['username']); $password = md5($_POST['password']); require_once("connection.php"); $result = mysql_query("SELECT * FROM table WHERE username='$username' AND password='$password'")or die(mysql_error()); if(mysql_num_rows($result) > 0) { $_SESSION['is_logged_in'] = 1; } } if(!isset($_SESSION['is_logged_in'])) { header("location:../login.php"); } else { header("location:../index.php"); } /* The following code converts $SESSION['username'] to the variable $_POST['username'] which is used to fill out the username value on the tipping form. */ if(mysql_num_rows($result) > 0) { $_SESSION['is_logged_in'] = 1; $_SESSION['username'] = $_POST['username']; } ?> Now that code will log me in, and a bad username/password redirects back to the login screen. If I use mysql_real_escape_string() around the $_POST variables, the result is that with the correct username and password, I am redirected back to the login page as if the login details were wrong. Why? $username = mysql_real_escape_string($_POST['username']); $password = mysql_real_escape_string(md5($_POST['password'])); I'd rather use a function, in a functions.php file which I'd include at the top of the page. I have been looking for something good, but whether good or bad, none of them worked. $_POST['username'] and $_POST['password'] don't like being wrapped in a function. Can anyone please suggest how I can modify this code so that it is sanitized AND works? Quote Link to comment Share on other sites More sharing options...
MadTechie Posted April 23, 2009 Share Posted April 23, 2009 make sure you connect to the database before you use mysql_real_escape_string Quote Link to comment Share on other sites More sharing options...
Potatis Posted April 23, 2009 Author Share Posted April 23, 2009 make sure you connect to the database before you use mysql_real_escape_string Ah thanks, I didn't know that I had to connect first. Maybe the functions will work now. Quote Link to comment Share on other sites More sharing options...
Potatis Posted April 23, 2009 Author Share Posted April 23, 2009 The login works with the functions, but it seems the functions don't work. I am testing with Firefox's sql inject me plug in and have just as many errors as without the function. Here is my functions.php: <?php function clean($str) { $str = trim($str); if(get_magic_quotes_gpc()) { $str = stripslashes($str); } return mysql_real_escape_string($str); } ?> And my updated login: <?php session_start(); if($_SERVER['REQUEST_METHOD'] == "POST") { include("functions.php"); require_once("connection.php"); $username = clean($_POST['username']); $password = clean(md5($_POST['password'])); $result = mysql_query("SELECT * FROM table WHERE username='$username' AND password='$password'")or die(mysql_error()); if(mysql_num_rows($result) > 0) { $_SESSION['is_logged_in'] = 1; } } if(!isset($_SESSION['is_logged_in'])) { header("location:../login.php"); } else { header("location:../index.php"); } /* The following code converts $SESSION['username'] to the variable $_POST['username'] which is used to fill out the username value on the tipping form. */ if(mysql_num_rows($result) > 0) { $_SESSION['is_logged_in'] = 1; $_SESSION['username'] = $_POST['username']; } ?> Is my login secure and the Firefox plugin is not right? Or does my script have problems? Quote Link to comment Share on other sites More sharing options...
MadTechie Posted April 23, 2009 Share Posted April 23, 2009 make sure you connect to the database before you use mysql_real_escape_string include("functions.php"); require_once("connection.php"); swap require_once("connection.php"); include("functions.php"); Quote Link to comment Share on other sites More sharing options...
Potatis Posted April 23, 2009 Author Share Posted April 23, 2009 Thanks again. I can login fine with the function now, I'm just unsure of how well it rates in terms of security. The Firefox plugin finds 51 Failures. :-\ Quote Link to comment Share on other sites More sharing options...
jonsjava Posted April 23, 2009 Share Posted April 23, 2009 how about this: <?php session_start(); function cleanInput($input){ /* EDIT THE FOLLOWING */ $db_user = "USERNAME"; $db_pass = "PASSWORD"; $db_host = "localhost"; $db_name = "DATABASE"; /* END EDIT */ $link2a3878sfw = mysql_connect($db_host, $db_user, $db_pass); mysql_select_db($db_name, $link2a3878sfw); if (is_array($input)){ foreach ($input as $key=>$value){ $output[$key] = mysql_real_escape_string($value); } } else{ $output = mysql_real_escape_string($value); } mysql_close($link2a3878sfw); return $output; } if($_SERVER['REQUEST_METHOD'] == "POST") { $post = cleanInput($_POST); $username = $post['username']; $password = md5($post['password']); require_once("connection.php"); $result = mysql_query("SELECT * FROM table WHERE username='$username' AND password='$password'")or die(mysql_error()); if(mysql_num_rows($result) > 0) { $_SESSION['is_logged_in'] = 1; } } if(!isset($_SESSION['is_logged_in'])) { header("location:../login.php"); } else { header("location:../index.php"); } /* The following code converts $SESSION['username'] to the variable $_POST['username'] which is used to fill out the username value on the tipping form. */ if(mysql_num_rows($result) > 0) { $_SESSION['is_logged_in'] = 1; $_SESSION['username'] = $post['username']; } ?> it's not glamorous, but it does the job. Quote Link to comment Share on other sites More sharing options...
Potatis Posted April 23, 2009 Author Share Posted April 23, 2009 Thanks, jonsjava, I'll give this a go now and let you know what Firefox says. Quote Link to comment Share on other sites More sharing options...
Potatis Posted April 23, 2009 Author Share Posted April 23, 2009 Well, I can log in fine. I still have 51 errors according to the plugin, but if you say this is secure, I'll stick with this. Thanks very much. Quote Link to comment Share on other sites More sharing options...
gffg4574fghsDSGDGKJYM Posted April 23, 2009 Share Posted April 23, 2009 If you post the error from the plugins it will help understand what the error actually is. Do you have other form on the same page ? the error may come from these. Quote Link to comment Share on other sites More sharing options...
Potatis Posted April 23, 2009 Author Share Posted April 23, 2009 Hi theonlydrayk, the login screen is a stand alone screen, there are no other forms. Here are the failures, which is code that the plugin injected into the login form: SQL Injection String Test Results username Submitted Form State: * password: * login: Submit Results: Server Status Code: 302 Moved Temporarily Tested value: 1 AND ASCII(LOWER(SUBSTRING((SELECT TOP 1 name FROM sysobjects WHERE xtype='U'), 1, 1))) > 116 Server Status Code: 302 Moved Temporarily Tested value: 1' OR '1'='1 Server Status Code: 302 Moved Temporarily Tested value: %31%27%20%4F%52%20%27%31%27%3D%27%31 Server Status Code: 302 Moved Temporarily Tested value: 1 UNION ALL SELECT 1,2,3,4,5,6,name FROM sysObjects WHERE xtype = 'U' -- Server Status Code: 302 Moved Temporarily Tested value: 1' OR '1'='1 Server Status Code: 302 Moved Temporarily Tested value: 1 UNI/**/ON SELECT ALL FROM WHERE Server Status Code: 302 Moved Temporarily Tested value: ' OR username IS NOT NULL OR username = ' Server Status Code: 302 Moved Temporarily Tested value: 1' AND non_existant_table = '1 Server Status Code: 302 Moved Temporarily Tested value: 1'1 Server Status Code: 302 Moved Temporarily Tested value: '; DESC users; -- Server Status Code: 302 Moved Temporarily Tested value: 1 AND USER_NAME() = 'dbo' Server Status Code: 302 Moved Temporarily Tested value: 1' AND 1=(SELECT COUNT(*) FROM tablenames); -- Server Status Code: 302 Moved Temporarily Tested value: 1 AND 1=1 Server Status Code: 302 Moved Temporarily Tested value: 1 EXEC XP_ Server Status Code: 302 Moved Temporarily Tested value: 1'1 Server Status Code: 302 Moved Temporarily Tested value: 1' OR '1'='1 Server Status Code: 302 Moved Temporarily Tested value: 1 OR 1=1 This field passed 14603 tests. To see all the passed results, go to Tools->SQL Inject Me->Options and click 'Show passed results in final report' and rerun this test. password Submitted Form State: * username: * login: Submit Results: Server Status Code: 302 Moved Temporarily Tested value: 1' OR '1'='1 Server Status Code: 302 Moved Temporarily Tested value: 1' OR '1'='1 Server Status Code: 302 Moved Temporarily Tested value: %31%27%20%4F%52%20%27%31%27%3D%27%31 Server Status Code: 302 Moved Temporarily Tested value: 1 UNI/**/ON SELECT ALL FROM WHERE Server Status Code: 302 Moved Temporarily Tested value: 1 UNION ALL SELECT 1,2,3,4,5,6,name FROM sysObjects WHERE xtype = 'U' -- Server Status Code: 302 Moved Temporarily Tested value: 1 AND ASCII(LOWER(SUBSTRING((SELECT TOP 1 name FROM sysobjects WHERE xtype='U'), 1, 1))) > 116 Server Status Code: 302 Moved Temporarily Tested value: ' OR username IS NOT NULL OR username = ' Server Status Code: 302 Moved Temporarily Tested value: 1' AND non_existant_table = '1 Server Status Code: 302 Moved Temporarily Tested value: 1'1 Server Status Code: 302 Moved Temporarily Tested value: '; DESC users; -- Server Status Code: 302 Moved Temporarily Tested value: 1 AND USER_NAME() = 'dbo' Server Status Code: 302 Moved Temporarily Tested value: 1' AND 1=(SELECT COUNT(*) FROM tablenames); -- Server Status Code: 302 Moved Temporarily Tested value: 1 AND 1=1 Server Status Code: 302 Moved Temporarily Tested value: 1 EXEC XP_ Server Status Code: 302 Moved Temporarily Tested value: 1'1 Server Status Code: 302 Moved Temporarily Tested value: 1' OR '1'='1 Server Status Code: 302 Moved Temporarily Tested value: 1 OR 1=1 This field passed 14603 tests. To see all the passed results, go to Tools->SQL Inject Me->Options and click 'Show passed results in final report' and rerun this test. login Submitted Form State: * username: * password: Results: Server Status Code: 302 Moved Temporarily Tested value: 1' OR '1'='1 Server Status Code: 302 Moved Temporarily Tested value: 1' OR '1'='1 Server Status Code: 302 Moved Temporarily Tested value: %31%27%20%4F%52%20%27%31%27%3D%27%31 Server Status Code: 302 Moved Temporarily Tested value: 1 UNI/**/ON SELECT ALL FROM WHERE Server Status Code: 302 Moved Temporarily Tested value: 1 UNION ALL SELECT 1,2,3,4,5,6,name FROM sysObjects WHERE xtype = 'U' -- Server Status Code: 302 Moved Temporarily Tested value: 1 AND ASCII(LOWER(SUBSTRING((SELECT TOP 1 name FROM sysobjects WHERE xtype='U'), 1, 1))) > 116 Server Status Code: 302 Moved Temporarily Tested value: ' OR username IS NOT NULL OR username = ' Server Status Code: 302 Moved Temporarily Tested value: 1' AND non_existant_table = '1 Server Status Code: 302 Moved Temporarily Tested value: 1'1 Server Status Code: 302 Moved Temporarily Tested value: '; DESC users; -- Server Status Code: 302 Moved Temporarily Tested value: 1 AND USER_NAME() = 'dbo' Server Status Code: 302 Moved Temporarily Tested value: 1' AND 1=(SELECT COUNT(*) FROM tablenames); -- Server Status Code: 302 Moved Temporarily Tested value: 1 AND 1=1 Server Status Code: 302 Moved Temporarily Tested value: 1 EXEC XP_ Server Status Code: 302 Moved Temporarily Tested value: 1'1 Server Status Code: 302 Moved Temporarily Tested value: 1' OR '1'='1 Server Status Code: 302 Moved Temporarily Tested value: 1 OR 1=1 This field passed 14603 tests. To see all the passed results, go to Tools->SQL Inject Me->Options and click 'Show passed results in final report' and rerun this test. Quote Link to comment Share on other sites More sharing options...
jonsjava Posted April 23, 2009 Share Posted April 23, 2009 Moved Temporarily just means that the server told it that the page had moved. Nothing to do with SQL injections. Quote Link to comment Share on other sites More sharing options...
Potatis Posted April 23, 2009 Author Share Posted April 23, 2009 Oh I see, it was listed as a failure, so I thought it must mean the script was vulnerable. Thanks for that. Quote Link to comment Share on other sites More sharing options...
jonsjava Posted April 23, 2009 Share Posted April 23, 2009 Glad to help! Quote Link to comment Share on other sites More sharing options...
gffg4574fghsDSGDGKJYM Posted April 23, 2009 Share Posted April 23, 2009 Moved Temporarily just means that the server told it that the page had moved. Nothing to do with SQL injections. Actually it might. The plugin assume the script work that way : login.php -> if the login is unsuccessfull don't redirect but show the same page with a login/password error message. -> if the login is successfull redirect the user to a profil/admin page. So when the plugin receive a redirect message it assume the SQL Injection work and the login is successfull. <?php if(!isset($_SESSION['is_logged_in'])) { header("location:../login.php"); } else { header("location:../index.php"); } Your code redirect the user if you are sucessfull or not without display any login error message, so the plugin cannot determine if it's actually a security issue or not and he right to display you a error or at least a warning that your site might be vulnerable to SQL Injection. The last script from jonsjava is safe from SQL Injection even if it's not glamorous Quote Link to comment Share on other sites More sharing options...
Potatis Posted April 23, 2009 Author Share Posted April 23, 2009 Moved Temporarily just means that the server told it that the page had moved. Nothing to do with SQL injections. Actually it might. The plugin assume the script work that way : login.php -> if the login is unsuccessfull don't redirect but show the same page with a login/password error message. -> if the login is successfull redirect the user to a profil/admin page. So when the plugin receive a redirect message it assume the SQL Injection work and the login is successfull. <?php if(!isset($_SESSION['is_logged_in'])) { header("location:../login.php"); } else { header("location:../index.php"); } Your code redirect the user if you are sucessfull or not without display any login error message, so the plugin cannot determine if it's actually a security issue or not and he right to display you a error or at least a warning that your site might be vulnerable to SQL Injection. The last script from jonsjava is safe from SQL Injection even if it's not glamorous Hi, thanks for the reply! I did investigate this further after jonsjava's last post. It was simply the redirect that was causing the 302 failures. I proved this by simply changing each one to Exit; to see which line caused the 302 failures. It was the unsuccessful login attempt. So changing the line that redirects back to the login page to "Exit;" the Firefox plugin finds no failures, and the page passes with flying green colours. Of course I want to keep the redirect back to the login page, and have to just accept that the plugin is reporting false positives. As for the code, I do have a functions.php file for the function, and an include, it is looking much neater now. 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.