This issue was originally posted to security.drupal.org, but we're making it public.

2nd factor authentication can be skipped in rare cases.

Steps to reproduce one possible scenario:

1. Enable TFA, TFA Basic
2. As a user with 'admin tfa settings' permission set up TFA with any default validation plugin and 'Recovery Codes' as fallback plugin.
3. Set up a user's TFA at 'user/[UID]/security/tfa'. Let that user be X from now on.
4. Use up 9 recovery codes with user X (tip: use a database editor).
5. In browser session "A" start logging in with user X. When TFA appears click on "Can't access your account?"
6. Use up the 10th recovery code in a separate browser session "B" or database editor.
7. In browser session "A" use the log in form to log in again.
8. X is logged in in browser session "A" skipping the whole TFA process.

This might look like a TFA Basic issue, but works with any plugin that can change from ready to not ready during a TFA process. The steps to reproduce this particular scenario are very unlikely to happen in real life, but it might be easier to catch this "window" with some other kind of plugin.

I'm happy to help fixing this issue.

Why this happens:

When one clicks on "Can't access your account?" the first fallback plugin (that is ready) takes the place of the validation plugin in the account's context. The fallback plugin's form is shown to the user.
->
The plugin somehow turns not ready.
->
User logs in again, request goes through tfa_login_submit(). .module line 271 TFA is not ready because the validation plugin is not ready.
->
User can log in without TFA.

CommentFileSizeAuthor
security-156702-7.patch891 bytesbanviktor

Comments

banviktor created an issue. See original summary.

coltrane’s picture

Thanks @banviktor for this report and patch. This is a security risk because a malicious might try to gain entry to an account using TFA by timing their attack while that person is in the TFA process. The risk is mitigated by the fact that an attacker must have the victims username and password, know the victim is logging in, and get them to be on a fallback plugin.

Tests pass and the patch looks good so I will be committing this and making a new beta release.

coltrane’s picture

Status: Needs review » Reviewed & tested by the community

  • coltrane committed 94b4c0a on 7.x-2.x authored by banviktor
    Issue #2628736 by banviktor: 2nd factor skippable when plugin turns from...
coltrane’s picture

Status: Reviewed & tested by the community » Fixed

Committed. beta3 forthcoming

justindodge’s picture

I'm just curious because apparently I don't fully understand the process, but why was this issue made public instead of going through the normal Drupal security channels and released along with other security announcements?
I thought that this was bad practice - is there some caveat that makes it permissible here?
Wouldn't users benefit from the public announcement of this issue so they can be aware there is an security vulnerability that they should fix on their sites? I know we certainly would have.
Thanks for any insight.

greggles’s picture

@justindodge I think that's covered in https://www.drupal.org/security-advisory-policy It's because of the release status of TFA/TFA_Basic. If you can work on the issues to make it stable (which are planned in #2241821: Plan for TFA 7.x-2.2 release) that will help get security coverage for this module.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.