steveo314 Posted August 31, 2017 Share Posted August 31, 2017 I don't know if this needs to be in this section or in the client side section... After 7 or 8 years of not having any issues, the php script that gets data from text boxes on an html "login" form on my website is not doing that. I check that the username has text right off the bat in the script and it comes back and empty. My server host has down updates like php5.6 to php7 recently and I have been modding code. But am still not getting anything from the html 'username' textbox. Has anyone else ran into this or is there something that needs to be updated or ... Quote Link to comment Share on other sites More sharing options...
requinix Posted August 31, 2017 Share Posted August 31, 2017 My bet. What's your code? Quote Link to comment Share on other sites More sharing options...
steveo314 Posted August 31, 2017 Author Share Posted August 31, 2017 (edited) the form and the php code are seperate files here is the form <form method="post" onsubmit="return vloginform(this);" action="https://www. .com/php_include/processlogin.php"> Username:<br> <input type="text" name="username" required><br> Password:<br> <input type="password" name="password" required><br> <input type="submit" value="login" name="submit" id="submit"> <input type="reset" value="reset" name="reset" id="reset"> </form> And here is the php username check: <?php session_start(); include("db_adapter.phpx"); if(!empty($_POST["submit"])) { $username = htmlentities($_POST["username"]); $password = htmlentities($_POST["password"]); $password = md5($password); $valid = checkinput($username,$password); if($valid == true) { $valid = checkuser($username,$password,$comp); if($valid == true) { startsession($username); if(isset($_POST["username"])) { header( 'refresh: 0; url=https://www. .com/customer_tools.php' ); } } else { $reasons = "Incorrect Username & Password Combination"; failed($reasons); } } else { $reasons = "User or password blank"; failed($reasons); } } function checkinput($username,$password) { if(strcmp($username,"") !== 0 or strcmp($password,"") !== 0) { return false; } else { return true; } } function checkuser($username,$password,$comp) { $checkquery = "SELECT username, password FROM users WHERE username='$username' AND password='$password'"; $result = mysqli_query($comp,$checkquery); $r_array = mysqli_fetch_array($result); if($r_array == "") { return false; } else { return true; } } function startsession($username) { $_POST["username"] = $username; return true; } function failed($reasons) { echo "<p>There are some problems with your login credentials:</p><p><b>$reasons</b></p>"; echo '<p><a href="https://www. .com/login.php">Continue Now</a>...</p>'; } ?> db_adapter.phpx is just a mysqli_connect command that ends up in $comp Edited August 31, 2017 by steveo314 Quote Link to comment Share on other sites More sharing options...
requinix Posted August 31, 2017 Share Posted August 31, 2017 aww Besides the insecure practices in there with the passwords, if(strcmp($username,"") !== 0 or strcmp($password,"") !== 0) {that's not right. Forget strcmp and just go with a regular comparison: if($username == "" or $password == "") { Quote Link to comment Share on other sites More sharing options...
steveo314 Posted August 31, 2017 Author Share Posted August 31, 2017 aww Besides the insecure practices in there with the passwords, if(strcmp($username,"") !== 0 or strcmp($password,"") !== 0) {that's not right. Forget strcmp and just go with a regular comparison: if($username == "" or $password == "") { is the md5 insecure now? Quote Link to comment Share on other sites More sharing options...
requinix Posted August 31, 2017 Share Posted August 31, 2017 "Now"? No. It's been insecure for a long time. Best practice now is to salt the password and hash it using a "slow" algorithm. In fact it's such an important thing to get right that PHP added built-in functions to deal with it. You could implement those changes if you wanted. Can't upgrade everyone's hashes all at once since you don't know the original passwords, but you can upgrade each person when they try to log in. It's barely any extra work if you're interested in doing it... which you should be... Quote Link to comment Share on other sites More sharing options...
steveo314 Posted August 31, 2017 Author Share Posted August 31, 2017 "Now"? No. It's been insecure for a long time. Best practice now is to salt the password and hash it using a "slow" algorithm. In fact it's such an important thing to get right that PHP added built-in functions to deal with it. You could implement those changes if you wanted. Can't upgrade everyone's hashes all at once since you don't know the original passwords, but you can upgrade each person when they try to log in. It's barely any extra work if you're interested in doing it... which you should be... I'm going to have to convert it then. I'm trying to update everything so that I don't have a dinosaur that I'm running on hopes and dreams. My biggest thing is trying to get the data from the html textboxes to php script. Quote Link to comment Share on other sites More sharing options...
requinix Posted August 31, 2017 Share Posted August 31, 2017 The username/password blank thing is likely due to the bug in checkinput so it should be working now... Updating the passwords is straightforward: if the password matches the old MD5 hash then you upgrade, otherwise you check with the new hash. Should go without saying that you'll need to adjust the place where people register too since that's where the password hash first gets saved. And similar places. Use password_hash to create a new password hash. It does the salting automatically. $hash = password_hash($password, PASSWORD_DEFAULT);Then store that value. You may need to update your users table first: make sure the password column is long enough to hold the hash, like VARCHAR(255). To compare the hash you can't use password_hash() again: the salt is random so trying again will give a different hash. Instead you use password_verify: if (password_verify($password_from_form, $hash_from_database)) {The magic is that the password hash contains not just a hash but information about how it was hashed: salt, algorithm, and any algorithm options that were used. When verifying, that information is extracted from the "hash" to replicate how the password was originally hashed, so when the new one for the password to verify is generated for comparison it will be identical. There's also password_needs_rehash, which is for future-proofing if the hash algorithm changes and is not actually useful yet. But since you're already doing something like this then you might as well code for it. if (password_needs_rehash($hash_from_database, PASSWORD_DEFAULT)) {You'd lump that in with the md5() check now. There's also a more significant security problem: SQL injection. htmlentities() isn't protecting you against someone entering an apostrophe into their username, and if they do it can cause havoc for your database and your site. That's even more important to fix than the hash thing. You've already switched to mysqli, which is where 95% of the work into fixing this would go. The other 5% is using prepared statements, which I'll demonstrate below. Do that and remove the htmlentities() stuff entirely. All together checkuser() looks like // $password is the original password from the form - not a hash! function checkuser($username,$password,$comp) { $checkquery = "SELECT username, password FROM users WHERE username=?"; // no password condition $checkstmt = mysqli_prepare($comp,$checkquery); $checkstmt->bind_param("s",$username); // $username into ? position $checkstmt->execute(); $row = $checkstmt->get_result()->fetch_array(); if (!$row) { // no user found return false; } $checkuser = false; $upgrade = false; // check new password hash if (password_verify($password,$row["password"])) { $checkuser = true; // check for upgrade if (password_needs_rehash($row["password"],PASSWORD_DEFAULT)) { $upgrade = true; } // check old md5 hash } else if (md5($password) == $row["password"]) { $checkuser = true; $upgrade = true; } if ($checkuser && $upgrade) { $hash = password_hash($password,PASSWORD_DEFAULT); $updatequery = "UPDATE users SET password=? WHERE username=?"; $updatestmt = mysqli_prepare($comp,$updatequery); $updatestmt->bind_param("s",$hash); // $hash into first ? $updatestmt->bind_param("s",$username); // $username into second ? $updatestmt->execute(); } return $checkuser; }That same $hash = password_hash($password,PASSWORD_DEFAULT);used when upgrading the hash would replace the $hash = md5($password);that you have wherever you store the password hash in the database - registration, change password form, reset password, whatever. Quote Link to comment Share on other sites More sharing options...
steveo314 Posted August 31, 2017 Author Share Posted August 31, 2017 I'm going to the 'die' on this script. This is the beginning of this one. <?php session_start(); if(!empty($_POST["username"])) { $username = $_POST["username"]; } else { die("username blank"); //$username = ""; } Quote Link to comment Share on other sites More sharing options...
requinix Posted August 31, 2017 Share Posted August 31, 2017 Is there anything in $_POST at all? var_dump($_POST);Is the REQUEST_METHOD correct? print_r($_SERVER["REQUEST_METHOD"]);What about print_r(file_get_contents("php://input"));If not then... this isn't the first time I've heard of this, or even second I think, but I don't quite remember what the cause was. Pretty sure it was weird. - Make sure there aren't any redirections happening. - What's the exact value of the post_max_size php.ini setting - ini_get("post_max_size") - Could the vloginform code have anything to do with this? - Apache or nginx? Quote Link to comment Share on other sites More sharing options...
steveo314 Posted August 31, 2017 Author Share Posted August 31, 2017 (edited) <?php session_start(); if(!empty($_POST["username"])) { $username = $_POST["username"]; } else { var_dump($_POST); print_r($_SERVER["REQUEST_METHOD"]); print_r(file_get_contents("php://input")); die("username blank"); //$username = ""; } outputs: array(0) { } GETusername blank I'm using Apache I'm still racking my brain as to what vloginform is Edited August 31, 2017 by steveo314 Quote Link to comment Share on other sites More sharing options...
requinix Posted August 31, 2017 Share Posted August 31, 2017 Looks like you've got a redirect happening somewhere. Does the form action action="https://www. .com/php_include/processlogin.php"exactly match what you see in the browser's address bar? Quote Link to comment Share on other sites More sharing options...
steveo314 Posted August 31, 2017 Author Share Posted August 31, 2017 (edited) Looks like you've got a redirect happening somewhere. Does the form action action="https://www. .com/php_include/processlogin.php"exactly match what you see in the browser's address bar? The web address is correct in my code. I omitted for use on here. The form starts off being on one php script, it jumps to the processlogin.php, and then it jumps to another php script The error comes from the code on post #11 which is the 3rd php script Edited August 31, 2017 by steveo314 Quote Link to comment Share on other sites More sharing options...
steveo314 Posted August 31, 2017 Author Share Posted August 31, 2017 login.php -> php_include/processlogin.php -> customer_tools.php Quote Link to comment Share on other sites More sharing options...
steveo314 Posted August 31, 2017 Author Share Posted August 31, 2017 Looks like you've got a redirect happening somewhere. Does the form action action="https://www. .com/php_include/processlogin.php"exactly match what you see in the browser's address bar? Do the $_POST arrays get lost with too many redirects now? Quote Link to comment Share on other sites More sharing options...
requinix Posted September 1, 2017 Share Posted September 1, 2017 They've always been lost. And they're lost with any redirects. It's an HTTP thing, not a PHP thing. Do all your processing in one script. If you want to separate the code then you can, but then you must include/require it from the first file. No redirects. Quote Link to comment Share on other sites More sharing options...
steveo314 Posted September 1, 2017 Author Share Posted September 1, 2017 They've always been lost. And they're lost with any redirects. It's an HTTP thing, not a PHP thing. Do all your processing in one script. If you want to separate the code then you can, but then you must include/require it from the first file. No redirects. I've been thinking about putting everything for that form in one script. Going to see if that would get me anywhere. 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.