Last Comment Bug 621110 - [SECURITY] Quips (adding/approving/deleting) lacks CSRF protection
: [SECURITY] Quips (adding/approving/deleting) lacks CSRF protection
Status: RESOLVED FIXED
[infrasec:csrf][ws:moderate]
:
Product: Bugzilla
Classification: Server Software
Component: Bugzilla-General (show other bugs)
: 3.6.3
: All All
: -- minor (vote)
: Bugzilla 3.2
Assigned To: Frédéric Buclin
: default-qa
:
Mentors:
Depends on:
Blocks: 835424 620540
  Show dependency treegraph
 
Reported: 2010-12-23 00:00 PST by Reed Loden [:reed] (use needinfo?)
Modified: 2013-01-28 10:08 PST (History)
2 users (show)
LpSolit: approval+
LpSolit: approval4.0+
LpSolit: blocking4.0+
LpSolit: approval3.6+
LpSolit: blocking3.6.4+
LpSolit: approval3.4+
LpSolit: blocking3.4.10+
LpSolit: approval3.2+
LpSolit: blocking3.2.10+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch for 4.1, v1 (3.25 KB, patch)
2010-12-27 13:27 PST, Frédéric Buclin
dkl: review+
Details | Diff | Splinter Review
patch for 3.2 - 4.0, v1 (3.26 KB, patch)
2011-01-22 10:51 PST, Frédéric Buclin
dkl: review+
Details | Diff | Splinter Review

Description User image Reed Loden [:reed] (use needinfo?) 2010-12-23 00:00:18 PST
quips.cgi has no CSRF protection.
Comment 1 User image Frédéric Buclin 2010-12-23 09:19:51 PST
Quips are really an unimportant bit of Bugzilla. You cannot do any harm.
Comment 2 User image Reed Loden [:reed] (use needinfo?) 2010-12-23 10:15:08 PST
Sure you can. You can add/approve/delete quips.
Comment 3 User image Frédéric Buclin 2010-12-23 10:32:51 PST
Yes, exactly what I said: you cannot do any harm. Quips are unimportant and have no interaction with other bits of the Bugzilla code.
Comment 4 User image Reed Loden [:reed] (use needinfo?) 2010-12-23 10:39:41 PST
I disagree with your assertion that you "cannot do any harm", and I think more than a few Bugzilla administrators would likewise disagree. Having the entire quips database deleted would definitely be harmful to Bugzilla users who actually use quips. Just because you don't use it doesn't mean it's not important to somebody else.
Comment 5 User image Frédéric Buclin 2010-12-27 13:27:41 PST
Created attachment 499887 [details] [diff] [review]
patch for 4.1, v1
Comment 6 User image Reed Loden [:reed] (use needinfo?) 2010-12-27 13:50:09 PST
Why hash tokens over session tokens here?
Comment 7 User image Frédéric Buclin 2010-12-27 13:53:15 PST
(In reply to comment #6)
> Why hash tokens over session tokens here?

Because I'm not going to fill the DB with tokens when editing quips.
Comment 8 User image David Lawrence [:dkl] 2011-01-10 17:03:51 PST
Comment on attachment 499887 [details] [diff] [review]
patch for 4.1, v1

Looks good and works as expected. r=dkl
Comment 9 User image Frédéric Buclin 2011-01-22 10:50:31 PST
Due to bug 398701 which landed on trunk only, I need to backport this patch on branches. A trivial change.
Comment 10 User image Frédéric Buclin 2011-01-22 10:51:41 PST
Created attachment 506126 [details] [diff] [review]
patch for 3.2 - 4.0, v1

replace FILTER uri by FILTER url_quote. This patch works on all branches, from 3.2 to 4.0.
Comment 11 User image David Lawrence [:dkl] 2011-01-22 15:39:57 PST
Comment on attachment 506126 [details] [diff] [review]
patch for 3.2 - 4.0, v1

Looks good and works as expected. r=dkl
Comment 13 User image Frédéric Buclin 2011-01-24 09:32:30 PST
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified quips.cgi
modified template/en/default/list/quips.html.tmpl
Committed revision 7670.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.0/
modified quips.cgi
modified template/en/default/list/quips.html.tmpl
Committed revision 7527.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/3.6/
modified quips.cgi
modified template/en/default/list/quips.html.tmpl
Committed revision 7222.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/3.4/
modified quips.cgi
modified template/en/default/list/quips.html.tmpl
Committed revision 6789.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/3.2/
modified quips.cgi
modified template/en/default/list/quips.html.tmpl
Committed revision 6410.
Comment 14 User image Frédéric Buclin 2011-01-24 17:20:06 PST
Security advisory sent. Removing the security flag.
Comment 15 User image Daniel Veditz [:dveditz] 2011-01-26 16:55:55 PST
Hard to imagine anything to do with quips rating ws:high

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