Last Comment Bug 621108 - [SECURITY] Creating/editing charts lacks CSRF protection
: [SECURITY] Creating/editing charts lacks CSRF protection
Status: RESOLVED FIXED
[infrasec:csrf][ws:moderate]
:
Product: Bugzilla
Classification: Server Software
Component: Reporting/Charting (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-22 23:33 PST by Reed Loden [:reed] (use needinfo?)
Modified: 2013-01-28 10:08 PST (History)
4 users (show)
LpSolit: approval+
LpSolit: approval4.0+
mkanat: blocking4.0+
LpSolit: approval3.6+
mkanat: blocking3.6.4+
LpSolit: approval3.4+
mkanat: blocking3.4.10+
LpSolit: approval3.2+
mkanat: blocking3.2.10+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch for 3.6 - 4.1, v1 (2.73 KB, patch)
2010-12-27 12:32 PST, Frédéric Buclin
dkl: review+
Details | Diff | Review
patch for 3.2 - 3.4, v1 (2.40 KB, patch)
2011-01-22 14:08 PST, Frédéric Buclin
no flags Details | Diff | Review
patch for 3.2 - 3.4, v1.1 (2.40 KB, patch)
2011-01-22 14:36 PST, Frédéric Buclin
dkl: review+
Details | Diff | Review

Description Reed Loden [:reed] (use needinfo?) 2010-12-22 23:33:15 PST
chart.cgi only supports tokens for deleting charts. Should also protect against unwanted chart creation/modification.
Comment 1 Max Kanat-Alexander 2010-12-22 23:37:56 PST
On the creating charts side, this is pretty minor. You'd be informed that you created a chart, and you'd just go delete it before it ever had a chance to gather data.

I suppose doing a CSRF on somebody and editing their charts would be a more significant annoyance.
Comment 2 Frédéric Buclin 2010-12-27 12:32:13 PST
Created attachment 499880 [details] [diff] [review]
patch for 3.6 - 4.1, v1
Comment 3 David Lawrence [:dkl] 2011-01-10 17:05:01 PST
Comment on attachment 499880 [details] [diff] [review]
patch for 3.6 - 4.1, v1

Looks good and works as expected. r=dkl
Comment 4 Frédéric Buclin 2011-01-22 11:24:18 PST
Backport needed for 3.4 and 3.2. Looks like a minor conflict in a template.
Comment 5 Frédéric Buclin 2011-01-22 11:25:26 PST
(In reply to comment #4)
> Looks like a minor conflict in a template.

err... in chart.cgi.
Comment 6 Frédéric Buclin 2011-01-22 14:08:30 PST
Created attachment 506149 [details] [diff] [review]
patch for 3.2 - 3.4, v1

It finally wasn't a minor conflict in chart.cgi. Bugzilla::Series objects in 3.4 and older have no ->id and ->name methods, and assertCanEdit() doesn't return a series object. This patch works for both 3.4 and 3.2.
Comment 7 Reed Loden [:reed] (use needinfo?) 2011-01-22 14:18:01 PST
Comment on attachment 506149 [details] [diff] [review]
patch for 3.2 - 3.4, v1

>-    my $series = new Bugzilla::Series($cgi);
>+    # We cannot use the $series objet below, because its name may have changed.

"object below, as its" (s/objet/object/ and s/because/as/)
Comment 8 Frédéric Buclin 2011-01-22 14:36:58 PST
Created attachment 506152 [details] [diff] [review]
patch for 3.2 - 3.4, v1.1

right, thanks!
Comment 10 David Lawrence [:dkl] 2011-01-23 20:34:19 PST
Comment on attachment 506152 [details] [diff] [review]
patch for 3.2 - 3.4, v1.1

Looks good and works as expected on 3.2/3.4
Comment 11 Frédéric Buclin 2011-01-24 09:20:21 PST
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified chart.cgi
modified template/en/default/reports/edit-series.html.tmpl
modified template/en/default/search/search-create-series.html.tmpl
Committed revision 7669.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.0/
modified chart.cgi
modified template/en/default/reports/edit-series.html.tmpl
modified template/en/default/search/search-create-series.html.tmpl
Committed revision 7526.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/3.6/
modified chart.cgi
modified template/en/default/reports/edit-series.html.tmpl
modified template/en/default/search/search-create-series.html.tmpl
Committed revision 7221.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/3.4/
modified chart.cgi
modified template/en/default/reports/edit-series.html.tmpl
modified template/en/default/search/search-create-series.html.tmpl
Committed revision 6788.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/3.2/
modified chart.cgi
modified template/en/default/reports/edit-series.html.tmpl
modified template/en/default/search/search-create-series.html.tmpl                       
Committed revision 6409.
Comment 12 Frédéric Buclin 2011-01-24 17:20:05 PST
Security advisory sent. Removing the security flag.

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