Jump to content

html to php help


steveo314

Recommended Posts

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 ...

Link to comment
Share on other sites

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 by steveo314
Link to comment
Share on other sites

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? 

Link to comment
Share on other sites

"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...

Link to comment
Share on other sites

"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. 

Link to comment
Share on other sites

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.
Link to comment
Share on other sites

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?

Link to comment
Share on other sites

<?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 by steveo314
Link to comment
Share on other sites

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 by steveo314
Link to comment
Share on other sites

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?

Link to comment
Share on other sites

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. 

Link to comment
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.