754673 - CSRF vulnerability in query.cgi allows possible unauthorized use of "Set my default search back to the system default"

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, defect)

Description

[laurens.bal]

Query.cgi is vulnerable to crsf.
The link "Set my default search back to the system default" has no protection against an crsf attack.

Comment 1

[laurens.bal]

Attachment: POC

Comment 2

[Reed Loden [:reed]]

Attachment: patch - v1

I think this should do it. Could move the token check under the if ($userid) check, but it doesn't really matter, as a token will always be generated, so might as well check it, even if it has no use when the user is not logged in.

Comment 3

[laurens.bal]

Does is this eligble for the bounty program?
Because it is a csrf vulnerability on bugzilla.mozilla.org --->

http://www.mozilla.org/security/bug-bounty-faq-webapp.html#eligible-bugs

Comment 4

[Reed Loden [:reed]]

(In reply to laurens.bal from comment #3)
> Does is this eligble for the bounty program?
> Because it is a csrf vulnerability on bugzilla.mozilla.org --->
> 
> http://www.mozilla.org/security/bug-bounty-faq-webapp.html#eligible-bugs

That's up to the folks who administrate the bounty program. You may contact security [@] mozilla.org to discuss that with them.

Either way, thank you for reporting this issue. We definitely appreciate your submission, and we welcome any future reports.

Comment 5

[Gervase Markham [:gerv]]

The impact of this is very low; having your default search reset would be classed as a "minor annoyance".

Gerv

Comment 7

[Daniel Veditz [:dveditz]]

Although a bug, a minor annoyance does not qualify for a web bounty

Comment 8

[laurens.bal]

I agree

Comment 9

[Frédéric Buclin]

Comment on attachment 623521 [details] [diff] [review]
patch - v1

>=== modified file 'query.cgi'

> if ($cgi->param('nukedefaultquery')) {
>+    my $token = $cgi->param('token');
>+    check_hash_token($token, ['nukedefaultquery']);
>     if ($userid) {

The token is only generated if userdefaultquery is true, i.e. only if the user is logged in. So it doesn't make sense to check the token for logged out users as nukedefaultquery has no effect for these users (and the error message would be confusing). So please move the token check inside |if ($userid)|. Otherwise your patch looks good.

Comment 10

[Reed Loden [:reed]]

Attachment: patch - v2

Comment 11

[Frédéric Buclin]

Comment on attachment 627507 [details] [diff] [review]
patch - v2

r=LpSolit

Comment 12

[Frédéric Buclin]

I don't want to break older installations by requiring a token for such a minor thing. So a=me for 4.2 and trunk only. You can commit this patch now and remove the security flag once it's committed.

Comment 13

[Reed Loden [:reed]]

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified query.cgi
modified template/en/default/search/knob.html.tmpl
Committed revision 8248.

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/4.2/
modified query.cgi
modified template/en/default/search/knob.html.tmpl
Committed revision 8092.