Closed Bug 1798673 Opened 2 years ago Closed 2 years ago

CSRF-login on bugzilla.mozilla.org

Categories

(bugzilla.mozilla.org :: General, defect)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: albinowax, Assigned: dkl)

References

()

Details

(Keywords: reporter-external, sec-moderate, wsec-csrf, Whiteboard: [reporter-external] [web-bounty-form] [verif?])

Attachments

(1 file, 3 obsolete files)

BMO has a bunch of code designed to prevent CSRF attacks from logging users into an attacker's account:
https://github.com/mozilla-bteam/bmo/blob/34a94dd51164776f6c142219bc0baa334a93084b/Bugzilla/Auth/Login/CGI.pm#L40-L69

Presumably this is to mitigate the risk of a victim not realising they've been logged into an attacker's account, and then posting security sensitive bugs. Unfortunately the MFA implementation is missing these defences, so a CSRF login is still possible.

To replicate this, first ensure MFA is enabled on your BMO account, then start a log in by providing your username and password. Record the 't' input from the MFA code form and close the tab. Simulate the victim by opening a fresh private-browsing window, and visiting a third party website with the following HTML:

<html>
<body>
<form action="https://bugzilla.mozilla.org/token.cgi" method="POST">
<input type="hidden" name="a" value="mfa_l" />
<input name="t" value="t-token-here" />
<input name="code" value="mfa-code-here" />
<input type="submit" value="Submit request" />
</form>
</body>
</html>

If you supply the 't' value and a valid MFA code, you'll find the 'victim' has been logged into your account.

Please note I also filed a different low-severity issue in BMO yesterday, but forgot to use this form. You can find it here: https://bugzilla.mozilla.org/show_bug.cgi?id=1798491

Flags: sec-bounty?
Group: websites-security → bugzilla-security
Type: task → defect
Component: Other → General
Product: Websites → bugzilla.mozilla.org

Hello James,

Thank you for your report.

I can confirm this behavior. The victim is logged into the attacker's account when supplying a valid token (t parameter) and mfa code.

Note for reference that the token is invalidated after a successful login. I am curious whether it also expires after a certain time.

Thanks,
Frida

Hello David,

Can you please take a look at this report? do you know how long the token generated after successful login remains valid?

Thanks,
Frida

Flags: needinfo?(dkl)

Thanks for confirming. From my perspective I don't think the 2FA login token lifetime has much impact on the security risk of this bug, since an attacker would automate generating this token server-side as soon as a victim landed on their site. They would generate the 2FA token in this way too. The victim would remain logged in until the session expires, which will be a very long time if the attacker checks the 'remember me' box in the initial step.

(In reply to Frida K [:frida] from comment #2)

Hello David,

Can you please take a look at this report? do you know how long the token generated after successful login remains valid?

Thanks,
Frida

MFA tokens (the 't' value) have shorter lives than other tokens which is 1 hour. Normal CSRF tokens from say the bug submission form, etc. last longer which is 3 days.

Flags: needinfo?(dkl)

(In reply to Jаmes Kettle from comment #3)

Thanks for confirming. From my perspective I don't think the 2FA login token lifetime has much impact on the security risk of this bug, since an attacker would automate generating this token server-side as soon as a victim landed on their site. They would generate the 2FA token in this way too. The victim would remain logged in until the session expires, which will be a very long time if the attacker checks the 'remember me' box in the initial step.

If we add a CSRF token along side the MFA token, then you would just need to add that as well to the HTML for the victim so that won't help either. One thing we could do is tie the MFA token to a specific IP address but not sure if that cannot be spoofed as well.

On thing is we could login the user first using their BMO session cookies before validating the MFA token. This way the cookies would need to match up with owner of the MFA token and if not then log them out. The cookies of the attacher is not something victim would have. I will see if this can fix the issue.

Looking into a fix for this.

Assignee: nobody → dkl
Status: NEW → ASSIGNED
Attached patch 1798673_1.patch (obsolete) — Splinter Review

This patch moves the token from the 2FA form to a cookie instead. This should fix the issue in that the cookie will not be available for verification if the form was generated from a different domain.

Attachment #9302457 - Flags: review?(glob)
Attached patch 1798673_2.patch (obsolete) — Splinter Review

Thanks for the reminder about other places that MFA is used. I did actually leave out some needed changes. This new patch should be more complete and I have tested each of the different paths that require MFA. Bad thing about using BMO for patch review is missing automated testing that we get with Github.

Attachment #9302457 - Attachment is obsolete: true
Attachment #9302457 - Flags: review?(glob)
Attachment #9302541 - Flags: review?(glob)
Comment on attachment 9302541 [details] [diff] [review] 1798673_2.patch Review of attachment 9302541 [details] [diff] [review]: ----------------------------------------------------------------- LGTM
Attachment #9302541 - Flags: review?(glob) → review+
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch 1798673_3.patch (obsolete) — Splinter Review

Sigh. The previous patch broke Duo MFA which regrettably wasnt tested as I could not set that up locally. The new patch should fix and I will test it on bugzilla-dev in the morning. I reverted this patch on production for now and will push the fixed version tomorrow.

Attachment #9302541 - Attachment is obsolete: true
Attachment #9302776 - Flags: review?(glob)
Attached patch 1798673_4.patchSplinter Review
Attachment #9302776 - Attachment is obsolete: true
Attachment #9302776 - Flags: review?(glob)
Attachment #9302780 - Flags: review?(glob)
Comment on attachment 9302780 [details] [diff] [review] 1798673_4.patch Review of attachment 9302780 [details] [diff] [review]: ----------------------------------------------------------------- r=glob
Attachment #9302780 - Flags: review?(glob) → review+

Revised version merged and deployed live which fixes Duo MFA along with TOTP
https://github.com/mozilla-bteam/bmo/commit/dd6ae6f79c05c9335f7d70fb9d639ca9dedde44f

Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED

Thanks David for the fix. I can no longer reproduce the issue using the PoC. I get error: User not specified, You must select/enter a user.

Thanks,
Frida

James: Thank you for reporting this issue. We are awarding a bounty as a "moderate" bug.

Flags: sec-bounty? → sec-bounty+

Thanks for the bounty and speedy fix, much appreciated. I'm happy for this to bug be disclosed whenever suits you.

Group: bugzilla-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: