Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

strong password not being hashed correctly #4004

Closed
Crytiqal opened this issue Nov 4, 2019 · 3 comments
Closed

strong password not being hashed correctly #4004

Crytiqal opened this issue Nov 4, 2019 · 3 comments
Assignees
Labels
type: bug A problem that should not be happening
Milestone

Comments

@Crytiqal
Copy link

Crytiqal commented Nov 4, 2019

Issue: special characters in password break the hash

Steps to reproduce:
I installed the latest released version (e107 V2.2.1) and during setup I used a randomly generated strong password for the Administrator account:

H?r}Fz^bT4N"`DU8

Upon completing the installation, I was unable to log in to the admin account. (Wrong password)

I changed the password in mysql via the cli to the md5 hash of "changeme" (4cb9c8a8048fd02294477fcb1a41191a) and was able to log into the admin account with this password.

I immediatly changed the password back to the originally intended strong password H?r}Fz^bT4N"`DU8 and again was not able to log back in and had to manually reset it back again to "changeme".

@Moc Moc added the status: testing required Someone needs to confirm this issue's existence and write a test to prevent the fix from regressing. label Nov 4, 2019
@Moc Moc added this to the e107 2.3.0 milestone Nov 4, 2019
@Moc Moc self-assigned this Nov 4, 2019
@Moc
Copy link
Member

Moc commented Nov 4, 2019

@Moc
Copy link
Member

Moc commented Nov 4, 2019

Confirmed.

This happens because " is transformed into " in one of the e107 methods (maybe toDb()?). The new password actually is

H?r}Fz^bT4N"`DU8

Doing more testing to see how we can fix this

Edit:
It is the e107::getParser()->filter() method. It uses FILTER_SANITIZE_STRING which will get rid of the double quotes. It is present in both /usersettings.php and probably called somewhere from install.php as well.

@CaMer0n Would it be safe to exempt the password field from this filter routine? And process it exactly as entered? Same as with the 'ue' fields?

Later on, the password will get hashed anyway.

E.g.

$ueVals   	= $_POST['ue'];
			$passtemp1 	= $_POST['password1'];
			$passtemp2  = $_POST['password2'];

			$_POST = e107::getParser()->filter($_POST);

			$_POST['ue'] 			= $ueVals;
			$_POST['password1']		= $passtemp1;
			$_POST['password2']    	= $passtemp2; 

Optionally we can also make use of FILTER_FLAG_NO_ENCODE_QUOTES which would require making changes to the e107::getParser()->filter() method.

@Moc Moc added type: bug A problem that should not be happening and removed status: testing required Someone needs to confirm this issue's existence and write a test to prevent the fix from regressing. labels Nov 4, 2019
@Moc Moc closed this as completed in 5b39b11 Nov 4, 2019
@Moc
Copy link
Member

Moc commented Nov 4, 2019

This has now been fixed. It is not the cleanest method, and perhaps we need to adjust the e107::getParser()->filter() method to include the FILTER_FLAG_NO_ENCODE_QUOTES flag but for now this works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A problem that should not be happening
Projects
None yet
Development

No branches or pull requests

2 participants