This project has moved and is read-only. For the latest updates, please go here.
1

Closed

Simultaneous login issue

description

The issue: UserRegistration object used during MFA auth is a server-wide global variable, and can be overwritten by another user logging in while auth is in progress.

Environment: latest release of adfsmfa from this repo, ADFS 3.0 @ Win2016, MSSQL database as data storage, all possible auth methods (sms, TOTP, mail) enabled and work properly.

Steps to reproduce:
  1. User A starts login process: enters username and password, selects SMS verification method, and sits still waiting for SMS to arrive.
  2. User B at some unrelated location starts login process at this moment (while user A is still waiting for SMS)
  3. At this moment, UserRegistration object which previously contained data of user A gets overwritten with user B data.
  4. Now user A can only login with verification credentials of user B. He logins into his own account, however.
    If he enters his received SMS code, verification will fail and user A may repeat it (successfully).
  5. Also, if user A selects "I don't have this code", he can view auth options of user B instead of his own.
The issue has been reproduced locally, and a pretty straightforward patch to fix the issue is attached.
There may be more correct ways to fix this, but said patch at least logs a warning into event log when the issue happens (and re-reads UserRegistration from the database for the correct user).

file attachments

Closed Feb 23 at 6:54 PM by redhook

comments

redhook wrote Feb 19 at 12:54 PM

Bug confirmed !
Security issue.
New release in few days.

redhook wrote Feb 20 at 4:57 PM

Hi KKorn
Thank you very much, for finding this bug in detailled way.
This bug is present since the first version (more than 1 year), and we are surprised that nobody find it before.
A new version is available (1.2.0.0).
We have made more complete refactoring, based on the informations you have provided.

Best regards

kkorn wrote Feb 20 at 7:10 PM

Thanks a lot for a quick turnaround.