Original report:
I ran into odd behavior when using D7.22, PRLP and Remember Me: when doing a password recovery as a regular authenticated user, you gain administrator access.
Steps to reproduce:
1. Install D7.22 + PRLP and Remember me module (latest stable versions)
2. Create a regular user
3. Login (with 'remember me' checked', not sure if necessary) and out as that user (not sure if whole step necessary)
4. Request a new password for that user
5. Click on the recover password link from the email
6. Set your new password (which the PRLP module provides)
7. After being logged in, you have administrator accessI've tried to hunt down the issue, and it appears to be the fault of the prlp_user_pass_reset_submit function. This function calls user_profile_form_submit($form, $form_state);, which in its turn calls entity_form_submit_build_entity('user', $account, $form, $form_state);, which copies all (non-field) top level form values from $form_state['values']. These values come from prlp_user_pass_reset_submit, and contains the following array:
$form_state['values']['roles'] = array( 2 => 2, 3 => 0 );I.e., no array_filter has been called on the array of roles for this user.
Next, the PRLP module calls user_login_finalize(), in order to invalidate the password reset token. This invokes hook_user_login(). Now, the Remember Me module implements this hook, and calls a user_save(), which then sets the roles for the user, and somehow does not call an array_filter() on them, and therefor assigns all available roles to the user, including administrator.
Calling array_filter() on the $form_state['values']['roles'] before calling user_profile_form_submit($form, $form_state); in the PRLP module fixes this behavior, but I think it should be sanitized in the core functions. Therefor I types this issue as both 'contrib' and 'core'.
Hope this helps.
subsequent analysis by larowlan:
The issue here is PRLP calls user_account_form which means all roles end up in form_state['values'] as the user_account_form is intended for selecting all roles.
Other calls to user_login_finalize would only use $user->roles which don't include inactive roles.
So I think PRLP should be responsible for filtering out these inactive roles.
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | 2081989-PRLP-security-fix.patch | 820 bytes | mike.roberts |
Comments
Comment #1
mike.roberts commentedI think this patch fixes the security issue.
Comment #2
mike.roberts commentedI've tested this a couple times now, on two different codebases and it seems to have fixed the issue. No more administrator. Can someone else review and test?
Comment #3
gregglesComment #4
jitesh doshi commentedThank you for the patch. I have included it in the latest beta2 release.