The default bug view has changed. See this FAQ.

[SECURITY] Creating/editing charts lacks CSRF protection

RESOLVED FIXED in Bugzilla 3.2

Status

()

Bugzilla
Reporting/Charting
--
minor
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: reed, Assigned: Frédéric Buclin)

Tracking

(Blocks: 1 bug)

3.6.3
Bugzilla 3.2
Dependency tree / graph
Bug Flags:
approval +
approval4.0 +
blocking4.0 +
approval3.6 +
blocking3.6.4 +
approval3.4 +
blocking3.4.10 +
approval3.2 +
blocking3.2.10 +

Details

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

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
chart.cgi only supports tokens for deleting charts. Should also protect against unwanted chart creation/modification.

Comment 1

6 years ago
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)

Updated

6 years ago
Assignee: charting → LpSolit
(Assignee)

Comment 2

6 years ago
Created attachment 499880 [details] [diff] [review]
patch for 3.6 - 4.1, v1
Attachment #499880 - Flags: review?(mkanat)
(Assignee)

Updated

6 years ago
Attachment #499880 - Flags: review?(mkanat) → review?(dkl)
(Reporter)

Updated

6 years ago
Flags: blocking4.0?
Flags: blocking3.6.4?
Flags: blocking3.4.10?
Flags: blocking3.2.10?
(Reporter)

Updated

6 years ago
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+

Updated

6 years ago
Flags: approval?
Flags: approval4.0?

Updated

6 years ago
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+
(Assignee)

Comment 4

6 years ago
Backport needed for 3.4 and 3.2. Looks like a minor conflict in a template.
Status: NEW → ASSIGNED
Flags: approval3.6?
(Assignee)

Comment 5

6 years ago
(In reply to comment #4)
> Looks like a minor conflict in a template.

err... in chart.cgi.
(Assignee)

Comment 6

6 years ago
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.
Attachment #506149 - Flags: review?(dkl)
(Reporter)

Comment 7

6 years ago
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/)
(Assignee)

Comment 8

6 years ago
Created attachment 506152 [details] [diff] [review]
patch for 3.2 - 3.4, v1.1

right, thanks!
Attachment #506149 - Attachment is obsolete: true
Attachment #506152 - Flags: review?(dkl)
Attachment #506149 - Flags: review?(dkl)
(Assignee)

Updated

6 years ago
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+

Updated

6 years ago
Flags: approval3.4?
Flags: approval3.2?
(Assignee)

Updated

6 years ago
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+
(Assignee)

Comment 11

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Comment 12

6 years ago
Security advisory sent. Removing the security flag.
Group: bugzilla-security
Whiteboard: [infrasec:csrf][ws:high] → [infrasec:csrf][ws:moderate]

Updated

4 years ago
Blocks: 835424
You need to log in before you can comment on or make changes to this bug.