0017338: Login_page.php: Ensure username is valid - MantisBT - Signup temporarily disabled due to spam

ID	Project	Category	View Status	Date Submitted	Last Update

0017338	mantisbt	security	public	2014-05-14 14:15	2014-12-22 08:21

Reporter	grangeway	Assigned To	dregad
Priority	normal	Severity	minor	Reproducibility	have not tried
Status	closed	Resolution	fixed
Target Version	1.2.18	Fixed in Version	1.2.18

Summary	0017338: Login_page.php: Ensure username is valid
Description	This is a fix to improve the behaviour of login_page against possible XSS exploits to ensure that a username is valid before displaying it back to the user when entered. Effectively, I believe this to be a false positive from a security scanning tool due to our use of string_attribute(), however as a defense in depth type of measure: a) we pass ?username= as a query param, back to logon page if a logon is failed b) if this is passed back, we use the data to complete the username field (assuming that the password is wrong) c) we then pass this directly into string_attribute() The patch contained against this bug tightens our handling of the username field by passing it to user_is_name_valid (P.S. that's a really annoying function name for a user's username when we also have real names :)) The user_is_name_valid function currently checks the length is not too long, and that it matches user_login_valid_regex which by default is alpha-numeric. For a user entering a reasonable attempt at a username and a password, the behaviour remains the same. For a user attempting to enter something like "<html style="foo">alert()" or similar, as this may exceed the username length and/or is likely to include invalid characters, the form will return to a blank username and password field to be completed, as if the user visited the logon page for the first time.
Tags	patch
Attached Files	17338.patch (449 bytes) diff --git a/login_page.php b/login_page.php index e00aee9..e32f195 100644 --- a/login_page.php +++ b/login_page.php @@ -102,6 +102,11 @@ if ( $t_session_validation ) { } } +# Check username is valid if provided +if( !user_is_name_valid( $f_username ) ) { + $f_username = ''; +} + # Determine whether the username or password field should receive automatic focus. $t_username_field_autofocus = 'autofocus'; $t_password_field_autofocus = ''; 17338.patch (449 bytes)

grangeway 2014-05-14 14:17 reporter ~0040283	Patch Attached, I've added this here as a private issue to ensure it can be evaluated properly in case my false positive analysis for an XSS issue is an incorrect determination

dregad 2014-05-15 05:12 developer ~0040287	I don't believe there is a vulnerability here, but anyway this can't hurt. I see no issue with this patch.

MantisBT: master-1.2.x d6e16b6f 2014-10-30 14:43 Paul Richards Committer: dregad Details Diff	Ensure username is valid in login_page.php This is a fix to improve the behaviour of login_page against possible XSS exploits to ensure that a username is valid before displaying it back to the user when entered. Fixes 0017338 Signed-off-by: Damien Regad <dregad@mantisbt.org>		Affected Issues 0017338
	mod - login_page.php		Diff File
MantisBT: master 3bb2bee6 2014-10-30 14:43 Paul Richards Committer: dregad Details Diff	Ensure username is valid in login_page.php This is a fix to improve the behaviour of login_page against possible XSS exploits to ensure that a username is valid before displaying it back to the user when entered. Fixes 0017338 Signed-off-by: Damien Regad <dregad@mantisbt.org>		Affected Issues 0017338
	mod - login_page.php		Diff File