encoding the password muddles which requirements are met [#2072171]

Passwords go through encodeURIComponent in the javascript and then are sanitized in password_policy_ajax_check with check_plain... the encoded password passes different tests than the real password.

Example: a password typed as '##' is encoded to '%23%23' (in the javscript) => and the password appears to have symbols, numbers, and four more characters than it really has. If check_plain did anything, it would probably increase the tests passed, like converting '>' to '<'

Possible fix: replace check_plain with rawurldecode in function password_policy_ajax_check()
$password = rawurldecode($_POST['password']);
I don't think this creates any security issues, a password should be able to have crazy characters, but I don't want to write a patch till someone confirms that. Is there another reason for check_plain?

Comment	File	Size	Author
#6	2072171-password-policy-encoding.patch	573 bytes	coltrane
#6
#1	password_policy-test-unencoded-password-2072171-1.patch	799 bytes	stephen Verdant
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

stephen Verdant CreditAttribution: stephen Verdant commented 27 August 2013 at 00:29

File	Size
password_policy-test-unencoded-password-2072171-1.patch	799 bytes

Since this fixes a minor problem while loosening what I *think* are unnecessary security efforts, another pair of eyes should review this carefully before using it.

Comment #2

erikwebb CreditAttribution: erikwebb commented 26 September 2013 at 13:33

Status:

Active

» Needs review

Comment #3

hefox CreditAttribution: hefox commented 16 October 2013 at 17:34

Is this on server side validation only, or can user submit passwords that are invalid?

Comment #4

erikwebb CreditAttribution: erikwebb commented 27 November 2013 at 15:44

Issue summary:	View changes
Status:	Needs review	» Needs work

Is this definitely not a problem for server-side?

Comment #5

hefox CreditAttribution: hefox commented 27 November 2013 at 16:21

Ack, Meant to ask, is it client side validation only, e.g. what is reported as warning?

Comment #6

coltrane

he/him

CreditAttribution: coltrane commented 28 November 2013 at 00:17

File	Size
2072171-password-policy-encoding.patch	573 bytes

This issue isn't about server-side bypassing, just about a discrepancy between the JS validation and the server-side validation.

I was able to reproduce this issue with a policy of 4 character constraint and nothing else. Then try to set a user's password to ## and the JS validation will accept it but when you go to submit the error will say there aren't enough characters.

Attached original report patch in .patch form.

Regarding the security aspect of this, check_plain() isn't providing much security since the password isn't being printed back out via the callback. If anything the form should set and JS should send a anti-CSRF token so the callback couldn't be used as a possible DOS vector. Also, Password Policy should do some amount of validation against bad input, for instance to protect against a form of arbitrary code execution by passing in user input into the policies -- but I think there's a low risk of this and it needs further research/thought.

Comment #7

hefox CreditAttribution: hefox commented 28 November 2013 at 01:33

To my understanding, CRSF only needed for DOS protection if the page uses more resources than a generic page (e.g. 'clear cache') or if an action is being done (e.g. 'block user link')

$_GET is url decoded already, $_POST isn't?