1273245 - [SECURITY] Backport upstream bug 1253263 to bmo/4.2 to fix XSS vulnerability in dependency graphs via bug summary

Status: RESOLVED FIXED

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

Description

[David Lawrence [:dkl]]

+++ This bug was initially created as a clone of Bug #1253263 +++

You can see this vulnerability in action on https://landfill.bugzilla.org/bugzilla-5.0-branch/showdependencygraph.cgi?id=32147&showsummary=1 or https://landfill.bugzilla.org/bugzilla-tip/showdependencygraph.cgi?id=28546&showsummary=1, clicking these links will currently display an alert saying "xss". This is because of the bug summary which has been set to the following string:

> rectangle (1,2) (3,4) http"><script>alert(/xss/)</script> 1

The issue is the code which read DOT output here: https://github.com/bugzilla/bugzilla/blob/86fb87f/showdependencygraph.cgi#L52. It assumes that each node will produce only one line, this isn't true however if the node label contains line breaks - dot will output the label at the end of the line and leave line breaks as they are. This is an issue when bug summary is being displayed because the label is something like "12345\nsummary" then - and "summary" becomes a line of its own in the resulting map file. Now all one has to do is making sure that the summary is accepted by the regexp on line 58 and add some malicious payload to the URL part (which will be inserted into HTML code without escaping). In fact, adding malicious code to the coordinates would have been possible as well.

Seeing bug 1221518, this code seems generally fragile - no escaping being performed for the values inserted into HTML output. Not sure why the the patch for bug 1221518 decided to escape a single field in an entirely different place, it would have been more logical to escape everything that's being inserted there (ideally by using proper templates).

Of course, the other part of the issue is Bugzilla using a deprecated map output format which cannot be parsed reliably.

Comment 1

[David Lawrence [:dkl]]

Attachment: 1273245_1.patch

Comment 2

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

Comment on attachment 8753035 [details] [diff] [review]
1273245_1.patch

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

r=dylan

Comment 3

[David Lawrence [:dkl]]

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   d63cbb5..3808736  master -> master

Comment 4

[David Lawrence [:dkl]]

pushed live