Closed Bug 621108 Opened 13 years ago Closed 13 years ago

[SECURITY] Creating/editing charts lacks CSRF protection

Categories

(Bugzilla :: Reporting/Charting, defect)

3.6.3
defect
Not set
minor

Tracking

()

RESOLVED FIXED
Bugzilla 3.2

People

(Reporter: reed, Assigned: LpSolit)

References

Details

(Whiteboard: [infrasec:csrf][ws:moderate])

Attachments

(2 files, 1 obsolete file)

chart.cgi only supports tokens for deleting charts. Should also protect against unwanted chart creation/modification.
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.
Severity: normal → minor
Assignee: charting → LpSolit
Attachment #499880 - Flags: review?(mkanat)
Attachment #499880 - Flags: review?(mkanat) → review?(dkl)
Flags: blocking4.0?
Flags: blocking3.6.4?
Flags: blocking3.4.10?
Flags: blocking3.2.10?
Blocks: 620540
Comment on attachment 499880 [details] [diff] [review]
patch for 3.6 - 4.1, v1

Looks good and works as expected. r=dkl
Attachment #499880 - Flags: review?(dkl) → review+
Flags: approval?
Flags: approval4.0?
Flags: blocking4.0?
Flags: blocking4.0+
Flags: blocking3.6.4?
Flags: blocking3.6.4+
Flags: blocking3.4.10?
Flags: blocking3.4.10+
Flags: blocking3.2.10?
Flags: blocking3.2.10+
Backport needed for 3.4 and 3.2. Looks like a minor conflict in a template.
Status: NEW → ASSIGNED
Flags: approval3.6?
(In reply to comment #4)
> Looks like a minor conflict in a template.

err... in chart.cgi.
Attached patch patch for 3.2 - 3.4, v1 (obsolete) — Splinter Review
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.
Attachment #506149 - Flags: review?(dkl)
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/)
right, thanks!
Attachment #506149 - Attachment is obsolete: true
Attachment #506152 - Flags: review?(dkl)
Attachment #506149 - Flags: review?(dkl)
Attachment #499880 - Attachment description: patch, v1 → patch for 3.6 - 4.1, v1
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
Attachment #506152 - Flags: review?(dkl) → review+
Flags: approval3.4?
Flags: approval3.2?
Flags: approval?
Flags: approval4.0?
Flags: approval4.0+
Flags: approval3.6?
Flags: approval3.6+
Flags: approval3.4?
Flags: approval3.4+
Flags: approval3.2?
Flags: approval3.2+
Flags: approval+
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.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Security advisory sent. Removing the security flag.
Group: bugzilla-security
Whiteboard: [infrasec:csrf][ws:high] → [infrasec:csrf][ws:moderate]
You need to log in before you can comment on or make changes to this bug.