697699 - (CVE-2011-3657) [SECURITY] XSS when viewing new charts or tabular and graphical reports in debug mode

Status: VERIFIED FIXED

(Bugzilla :: Reporting/Charting, defect)

Description

[Patrick Hof]

RedTeam Pentesting discovered a Cross Site-Scripting (XSS) vulnerability
in Bugzilla's chart generator during a penetration test.  If attackers
can persuade users to click on a prepared link or redirected them to
such a link from an attacker-controlled website, they are able to run
arbitrary JavaScript code in the context of the Bugzilla installation's
domain.


Details
=======

Product: Bugzilla
Affected Versions: 3.4.12, 3.6.6, 4.0.2, 4.1.3,
                   possibly all older versions that can generate charts
Vulnerability Type: Cross Site Scripting
Security Risk: high
Vendor URL: http://www.bugzilla.org
Vendor Status: notified
Advisory Status: private


More Details
============

The chart-generating script chart.cgi contains a function plot(), which
creates a new chart:

sub plot {
    validateWidthAndHeight();
    $vars->{'chart'} = new Bugzilla::Chart($cgi);

    my $format = $template->get_format("reports/chart", "", scalar($cgi->param('ctype')));

    # Debugging PNGs is a pain; we need to be able to see the error messages
    if ($cgi->param('debug')) {
        print $cgi->header();
        $vars->{'chart'}->dump();
    }

    print $cgi->header($format->{'ctype'});
    disable_utf8() if ($format->{'ctype'} =~ /^image\//);

    $template->process($format->{'template'}, $vars)
      || ThrowTemplateError($template->error());
}

The function's code shows that there is a "debug" parameter, that, if
set, will make the function print out the variables (and their contents)
representing the chart for debugging purposes. This includes
user-defined variables sent as parameters, especially "label0". As the
content of this variable is not checked for malicious input, it can be
used to inject arbitrary JavaScript code into the debugging output. In
fact, any variable of the form "labelXXX", where "XXX" is an arbitrary
number, will work.


Proof of Concept
================

The following URL generates a new chart with debugging output enabled,
containing JavaScript code in the "label0" parameter:

http://www.example.org/bugzilla/chart.cgi
  ?category=-All-
  &datefrom=
  &dateto=
  &label0=<script>alert("XSS")</script>
  &line0=1
  &name=1
  &subcategory=-All-
  &ctype=png
  &action=plot
  &width=600
  &height=350
  &debug=1


Security Risk
=============

The risk of this vulnerability is estimated to be high. Being able to
embed arbitrary JavaScript allows attackers to completely manipulate the
website, adding one's own content and tracking user interaction.


RedTeam Pentesting GmbH
=======================

RedTeam Pentesting offers individual penetration tests, short pentests,
performed by a team of specialised IT-security experts. Hereby, security
weaknesses in company networks or products are uncovered and can be
fixed immediately.

As there are only few experts in this field, RedTeam Pentesting wants to
share its knowledge and enhance the public knowledge with research in
security related areas. The results are made available as public
security advisories.

More information about RedTeam Pentesting can be found at
http://www.redteam-pentesting.de.

Comment 1

[:glob ✱]

Attachment: patch v1

confirming; fix attached.

Comment 2

[Gervase Markham [:gerv]]

Hey guys,

Thanks for reporting this. I wrote that code, quite some time ago :-| We have various protections in place to detect possible XSS for output coming from templates, but obviously if you just write "print" in a CGI, then... <sigh>

Gerv

Comment 3

[Gervase Markham [:gerv]]

Comment on attachment 569947 [details] [diff] [review]
patch v1

r=gerv.

Gerv

Comment 4

[Patrick Hof]

Sure, glad to help.

Our client also suggested that it'd be great if the hardcoded debugging output (there's more in other places, as I guess you know) could be made optional, as it's normally not needed in a production system and might only lead to problems like this. We didn't find any other problems in the debugging code, but it's always good to reduce your attack surface.

Comment 5

[Gervase Markham [:gerv]]

I just did a quick audit of all calls to "print" and "say" in the current codebase, and I've found one more confirmed instance of this - in report.cgi, lines 288-294. Same type of code, put there for the same reason, by the same person, with the same bug. :-| You can trigger it in that case by updating your Real Name to have a <script> tag in it, making e.g. Assignee Real Name one of the chart parameters, and then putting debug=1 on the end of your chart URL.

Gerv

Comment 6

[Gervase Markham [:gerv]]

By "quick audit", I of course mean "possibly incomplete and should be repeated more thoroughly". I also did a search for other uses of Data::Dumper but couldn't immediately find any vulnerabilities.

Gerv

Comment 7

[:glob ✱]

Attachment: patch for 4.2 and older, v2

fixes the report.cgi instance.
i was also unable to find any other occurrences that smelt bad.

Comment 8

[Gervase Markham [:gerv]]

Comment on attachment 569963 [details] [diff] [review]
patch for 4.2 and older, v2

r=gerv.

Gerv

Comment 9

[Frédéric Buclin]

I guess this patch applies to all branches?

Comment 10

[:glob ✱]

(In reply to Frédéric Buclin from comment #9)
> I guess this patch applies to all branches?

yes, except for trunk.

Comment 11

[:glob ✱]

Attachment: patch v2 (trunk)

Comment 12

[Gervase Markham [:gerv]]

Comment on attachment 570593 [details] [diff] [review]
patch v2 (trunk)

r=gerv.

Gerv

Comment 13

[Max Kanat-Alexander]

Thanks for reporting, fixing, and reviewing this, everybody. I'm a little pressed for time until after Wednesday, but I will start planning to do a sec release for this shortly after that. If anybody else wants to file the relevant release bugs and do the sec adv, it would definitely speed up the process.

Comment 14

[Patrick Hof]

Hi,

as the last update is almost a month old: what's your current time schedule for releasing the fix? Any idea when this'll be officially announced?

Comment 15

[Frédéric Buclin]

(In reply to Patrick Hof from comment #14)
> what's your current time schedule
> for releasing the fix? Any idea when this'll be officially announced?

We have no hard release date as we are still tracking some other blockers, but this should happen within a few weeks (I would say mid-December at the lastest, without guarantee). The patches in this bug will be committed right before the releases.

Comment 16

[Daniel Veditz [:dveditz]]

Assigning CVE-2011-3657 to this issue.

Comment 17

[David Lawrence [:dkl]]

3.4:
Committing to: bzr+ssh://[e-mail]/bugzilla/3.4
modified report.cgi
modified Bugzilla/Chart.pm
Committed revision 6817. 

3.6:
Committing to: bzr+ssh://[e-mail]/bugzilla/3.6
modified report.cgi
modified Bugzilla/Chart.pm
Committed revision 7266.

4.0:
Committing to: bzr+ssh://[e-mail]/bugzilla/4.0
modified report.cgi
modified Bugzilla/Chart.pm
Committed revision 7671.

4.2:
Committing to: bzr+ssh://[e-mail]/bugzilla/4.2
modified report.cgi
modified Bugzilla/Chart.pm
Committed revision 7990.

trunk:
Committing to: bzr+ssh://[e-mail]/bugzilla/trunk
modified report.cgi
modified Bugzilla/Chart.pm
Committed revision 8053.

dkl

Comment 18

[Frédéric Buclin]

Security Advisory sent and is live on bugzilla.org. Removing the security flag.

Comment 19

[Mark Goodwin [:mgoodwin]]

Thanks