1252445 - Tracking flags configuration is vulnerable to CSRF and causes persistent XSS

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: Extensions, defect)

Description

[Wladimir Palant]

Tracking flags configuration has no CSRF protection currently meaning that anybody who successfully lures a Bugzilla admin to a malicious page can add or change tracking flags. For example, a malicious page could contain the following code:

> <img src="https://bugzilla-dev.allizom.org/page.cgi?id=tracking_flags_admin_edit.html&mode=new&values=[{%22comment%22%3A%22%22%2C%22is_active%22%3Atrue%2C%22setter_group_id%22%3A%221%22%2C%22value%22%3A%22---%22%2C%22id%22%3A0}]&visibility=[{%22component%22%3A%22%22%2C%22product%22%3A%22Firefox%22%2C%22id%22%3A0}]&save=1&flag_name=cf_foobar%3C/script%3E%3Cscript%3Ealert%28/xss/%29%3C/script%3E&flag_desc=foobar&flag_type=project&flag_sort=0&flag_enter_bug=1&flag_active=1">

Not only will this create a new tracking flag, anybody visiting a Firefox bug after that will see a message saying "xss" - the tracking flags data is being inserted into a script as JSON, without further validation. So one can add XSS payload to the flag ID and turn this into persistent XSS (there is client-side validation for flag ID but almost nothing on the server side).

Interesting fact: setting flag ID to something like "cf_foo::bar" will cause an error message whenever a bug is saved because the following line will try calling method bar via package cf_foo:

> my $new_value = $bug->$flag_name;

(extensions/TrackingFlags/Extension.pm line 650)

From what I can tell, this is not exploitable because there are no packages with names starting with cf_.

Comment 1

[David Lawrence [:dkl]]

Attachment: 1252445_1.patch

Comment 2

[Wladimir Palant]

Comment on attachment 8725270 [details] [diff] [review]
1252445_1.patch

Is that the right variables you escaped? The problematic line here was |TrackingFlags = {...}|.

Comment 3

[Wladimir Palant]

Comment on attachment 8725270 [details] [diff] [review]
1252445_1.patch

Yes, the XSS that I observed was in extensions/TrackingFlags/template/en/default/hook/bug/edit-after_custom_fields.html.tmpl:

>   TrackingFlags = [% tracking_flags_json FILTER none %];

It was popping up in each bug rather than the flags edit page.

Comment 4

[David Lawrence [:dkl]]

(In reply to Wladimir Palant from comment #3)
> Comment on attachment 8725270 [details] [diff] [review]
> 1252445_1.patch
> 
> Yes, the XSS that I observed was in
> extensions/TrackingFlags/template/en/default/hook/bug/edit-
> after_custom_fields.html.tmpl:
> 
> >   TrackingFlags = [% tracking_flags_json FILTER none %];
> 
> It was popping up in each bug rather than the flags edit page.

Ah thanks for the heads up. Will submit new patch with the added fix.

dkl

Comment 5

[David Lawrence [:dkl]]

Attachment: 1252445_2.patch

Comment 6

[Wladimir Palant]

extensions/BMO/template/en/default/pages/release_tracking_report.html.tmpl looks like it would also be vulnerable in this context (particularly assigning to fields_data variable). However, it requires some rather complicated setup so I cannot verify.

Comment 7

[Dylan Hardison [:dylan] (he/him)]

(In reply to Wladimir Palant from comment #6)
> extensions/BMO/template/en/default/pages/release_tracking_report.html.tmpl
> looks like it would also be vulnerable in this context (particularly
> assigning to fields_data variable). However, it requires some rather
> complicated setup so I cannot verify.

There is already a bug filed for that, in fact. It'll be in the next release.

Comment 8

[Dylan Hardison [:dylan] (he/him)]

Comment on attachment 8725376 [details] [diff] [review]
1252445_2.patch

Review of attachment 8725376 [details] [diff] [review]:
-----------------------------------------------------------------

r=dylan

Comment 9

[David Lawrence [:dkl]]

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   0a9f058..02aa6ce  master -> master

Comment 10

[David Lawrence [:dkl]]

Pushed