Last Comment Bug 706271 - CSRF vulnerability in token.cgi allows possible unauthorized password reset e-mail request
: CSRF vulnerability in token.cgi allows possible unauthorized password reset e...
Status: RESOLVED FIXED
[infrasec:csrf][ws:low]
: sec-low
Product: Bugzilla
Classification: Server Software
Component: User Accounts (show other bugs)
: unspecified
: All All
: -- minor (vote)
: Bugzilla 4.2
Assigned To: Frédéric Buclin
: default-qa
Mentors:
: 772809 (view as bug list)
Depends on:
Blocks: 850126
  Show dependency treegraph
 
Reported: 2011-11-29 14:36 PST by Mario Gomes
Modified: 2013-03-12 04:34 PDT (History)
5 users (show)
LpSolit: approval+
LpSolit: approval4.2+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch - v1 (1.70 KB, patch)
2012-02-17 11:49 PST, Reed Loden [:reed] (use needinfo?)
LpSolit: review-
Details | Diff | Splinter Review
patch, v2 (3.30 KB, patch)
2012-08-05 16:38 PDT, Frédéric Buclin
reed: review+
Details | Diff | Splinter Review
patch for 4.2, v1 (2.64 KB, patch)
2012-08-06 10:20 PDT, Frédéric Buclin
reed: review+
Details | Diff | Splinter Review

Description Mario Gomes 2011-11-29 14:36:00 PST
User Agent: Mozilla/5.0 (Windows NT 6.1) AppleWebKit/535.8 (KHTML, like Gecko) Chrome/17.0.942.0 Safari/535.8

Steps to reproduce:

Well, i'm reporting only because, this can do spam. But, risk to security don't have any.
Guys, if you want fix, there are the demo: https://landfill.bugzilla.org/bugzilla-tip/token.cgi?a=reqpw&loginname=demo@csrf.com
Comment 1 Max Kanat-Alexander 2011-11-29 22:02:57 PST
Yeah, this is pretty minor. But we should still hash-token it the same way we did for createaccount.
Comment 2 Reed Loden [:reed] (use needinfo?) 2012-02-17 11:49:19 PST
Created attachment 598315 [details] [diff] [review]
patch - v1
Comment 3 Frédéric Buclin 2012-02-18 11:28:02 PST
Comment on attachment 598315 [details] [diff] [review]
patch - v1

If someone accesses token.cgi directly, it will now get the "Suspicious Action" confirmation page (which I can live with, though I still don't think this bug must be fixed). But if the user clicks the confirmation button, it fails complaining that the page doesn't exist.
Comment 4 Frédéric Buclin 2012-07-22 08:07:33 PDT
*** Bug 772809 has been marked as a duplicate of this bug. ***
Comment 5 Frédéric Buclin 2012-07-30 13:32:46 PDT
Please leave this bug alone. Just because a bug is not fixed immediately doesn't mean it will never be fixed. And if we decide it's minor enough to not be fixed, we will close it as wontfix, not invalid.
Comment 6 Mario Gomes 2012-07-30 13:35:34 PDT
lol...
Comment 7 Frédéric Buclin 2012-08-05 16:38:32 PDT
Created attachment 649151 [details] [diff] [review]
patch, v2

The reason of the error I mentioned in comment 3 is because both global/confirm-action.html.tmpl and account/auth/login-small.html.tmpl use a variable named "script_name", and so login-small.html.tmpl was overwritting the value passed to confirm-action.html.tmpl. The fix is to simply rename the variable in login-small.html.tmpl to something else.
Comment 8 Reed Loden [:reed] (use needinfo?) 2012-08-05 17:08:38 PDT
Comment on attachment 649151 [details] [diff] [review]
patch, v2

r=reed

works as expected, and direct access to token.cgi with loginname=email@address.tld&a=reqpw shows the suspicious action page, and I am able to continue passed it fine.
Comment 9 Frédéric Buclin 2012-08-05 17:29:49 PDT
I will have to backport the patch to 4.2 as token.cgi is very different between 4.2 and 4.4.
Comment 10 Frédéric Buclin 2012-08-06 10:20:01 PDT
Created attachment 649311 [details] [diff] [review]
patch for 4.2, v1

Backport for 4.2. No code change, only fixed a conflict in token.cgi.
Comment 11 Reed Loden [:reed] (use needinfo?) 2012-08-06 12:32:23 PDT
Comment on attachment 649311 [details] [diff] [review]
patch for 4.2, v1

Technically, you're doing the check in token.cgi in a different spot than trunk, but not sure it really matters. Though, I suspect it makes more sense to get the super-basic checks done (like valid e-mail address format) before doing token check (which can take more perf), but whatever...
Comment 12 Frédéric Buclin 2012-08-06 14:46:07 PDT
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified token.cgi
modified template/en/default/account/auth/login-small.html.tmpl
modified template/en/default/account/auth/login.html.tmpl
Committed revision 8330.


Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.2/
modified token.cgi
modified template/en/default/account/auth/login-small.html.tmpl
modified template/en/default/account/auth/login.html.tmpl
Committed revision 8114.

Note You need to log in before you can comment on or make changes to this bug.