? Success

User tests: Successful: Unsuccessful:

avatar jo-sf
jo-sf
6 Dec 2015

If the user supplies either an unknown user name or a wrong password or a wrong secret key the com_users login form is shown (probably again) and it contains not only the user name typed in but also the password and (if activated) the secret key. The latter two should never be sent back to the user's browser for security reasons as is the case with the registration form when you enter e.g. a user name already in use or an email address already used with any other registration.

Test instructions: locate the login form at any page in the frontend and enter any invalid login information. After sending this data to the server you'll be shown the com_users login form containing the login information you just entered (the password field contains only dots but when you look into the HTML code you'll see the password you entered in clear text).

avatar jo-sf jo-sf - change - 6 Dec 2015
Status New Pending
avatar jo-sf jo-sf - open - 6 Dec 2015
avatar joomla-cms-bot joomla-cms-bot - change - 6 Dec 2015
Labels Added: ?
avatar PhilETaylor
PhilETaylor - comment - 6 Dec 2015

Good catch - Tested and the patch works well.

This is NOT a security issue however, as the data sent back is not the REAL password, but the data you sent to the server anyway.

avatar jo-sf
jo-sf - comment - 6 Dec 2015

Well - yes and no. It might be a real password, e.g. in case you mistyped the user name - or maybe even more dangerous: both the user name and the password might be correct, but the secret key is not. And this login form might be shown using HTTP even if you enabled sending the login data using HTTPS ("usesecure" setting in your mod_login instance).

avatar PhilETaylor
PhilETaylor - comment - 6 Dec 2015

if you are sending to a https then the resultant error page is also on https - I still fail to see how this is an exploitable security issue. You are sat infront of your pc and you provide the credentials and they are displayed back to you... no one else, and not pulled from the db or anything like that - Garbage in Garbage Out.

MANY web applications return your failed password if you fail to login, so that you can change the typo you made.

However I agree removing it from the error page is a good step and I recommend this is merged.

avatar mbabker
mbabker - comment - 6 Dec 2015

For future reference:

Report security issues to the Joomla! Security Strike Team (JSST) at security@joomla.org or use the JSST contact form. Please do not use the public tracker for security issues.

https://github.com/joomla/joomla-cms/blob/staging/CONTRIBUTING.md

avatar infograf768
infograf768 - comment - 10 Dec 2015

I guess Travis should be relaunched here.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8604.

avatar jo-sf
jo-sf - comment - 10 Dec 2015

@infograf768
Why do you think so? Is there something wrong with my change?

The first Travis check failed in the PHP:7 check, but this was as far as I've seen not a problem with my change but it was a problem with Travis itself: something about pecl being used but not supported. I saw this and I found that it was fixed some time later within Travis (at least I think the fix I've seen was due to that pecl problem).

In order to rerun Travis I made another change by adding a comment to my patch which was nevertheless good to do. After committing this second change Travis was relaunched automatically, and this time it succeeded for all PHP versions.

avatar infograf768
infograf768 - comment - 10 Dec 2015

Yes, it was a Travis issue but I think it is now solved. :+1:

avatar andrepereiradasilva andrepereiradasilva - test_item - 3 Jan 2016 - Tested successfully
avatar andrepereiradasilva
andrepereiradasilva - comment - 3 Jan 2016

I have tested this item :white_check_mark: successfully on 06ffe93

One thing i notice after this patch is the behaviour is slightly different in frontend login and backend login.

frontend login: on fail the user remains in the form, but not the password
backend login: on fail, neither the user nor the password remains in the form


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8604.

avatar jo-sf
jo-sf - comment - 4 Jan 2016

@andrepereiradasilva

Since my patch only addresses the com_users login form I assume that the different behaviour with the backend login already existed before my patch. As far as I've seen in the source code the backend login form is shown again (without any data filled in) if the supplied user name or password is wrong.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8604.

avatar infograf768
infograf768 - comment - 4 Jan 2016

I suggest to add
$data['username'] = ''; to your PR. This will clear the Username field on failure.

avatar andrepereiradasilva
andrepereiradasilva - comment - 4 Jan 2016

@jo-sf. Yes, i know that was not to do with your PR. As you noticed i tested this PR successfully.

The difference in behaviour between frontend and backend login was already there before your PR. With your PR they are more likely, just the username part is different.

I just said that because IMHO all login form should have equal behaviors in that reggard.

avatar waader waader - test_item - 5 Jan 2016 - Tested successfully
avatar waader
waader - comment - 5 Jan 2016

I have tested this item :white_check_mark: successfully on 06ffe93


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8604.

avatar joomla-cms-bot
joomla-cms-bot - comment - 5 Jan 2016

This PR has received new commits.

CC: @andrepereiradasilva, @waader


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8604.

avatar jo-sf
jo-sf - comment - 5 Jan 2016

As @infograf768 and @andrepereiradasilva suggested I changed the frontend behaviour in case of a failure: now the user name will be cleared too.

avatar waader
waader - comment - 5 Jan 2016

Works! Thanks jo-sf.

avatar andrepereiradasilva andrepereiradasilva - test_item - 5 Jan 2016 - Tested successfully
avatar andrepereiradasilva
andrepereiradasilva - comment - 5 Jan 2016

I have tested this item :white_check_mark: successfully on 77f0d81


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8604.

avatar infograf768 infograf768 - test_item - 6 Jan 2016 - Tested successfully
avatar infograf768
infograf768 - comment - 6 Jan 2016

I have tested this item :white_check_mark: successfully on 77f0d81


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8604.

avatar infograf768 infograf768 - change - 6 Jan 2016
Status Pending Ready to Commit
avatar infograf768
infograf768 - comment - 6 Jan 2016

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8604.

avatar joomla-cms-bot joomla-cms-bot - change - 6 Jan 2016
Labels Added: ?
avatar rdeutz
rdeutz - comment - 6 Jan 2016

not sure if we should include it in 3.5.0 or 3.5.1 something for the release leader to decide @roland-d

avatar roland-d roland-d - change - 6 Jan 2016
Milestone Added:
avatar roland-d roland-d - change - 6 Jan 2016
Status Ready to Commit Closed
Closed_Date 0000-00-00 00:00:00 2016-01-06 11:55:38
Closed_By roland-d
avatar roland-d roland-d - reference | 75adb40 - 6 Jan 16
avatar roland-d roland-d - merge - 6 Jan 2016
avatar roland-d roland-d - close - 6 Jan 2016

Add a Comment

Login with GitHub to post a comment