1253267 - DOT injection vulnerability in dependency graphs when long bug summaries are wrapped

Status: RESOLVED FIXED

(Bugzilla :: Dependency Views, defect)

Description

[Wladimir Palant]

See https://landfill.bugzilla.org/bugzilla-tip/showdependencygraph.cgi?id=28547&showsummary=1 - this graph will show the bug in blue which isn't supposed to happen. Also, the default URL for the image map has been set to "malicious". That's because of the bug summary:

> longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong [color=blue,URL=malicious]

The long prefix causes the label to be wrapped here: https://github.com/bugzilla/bugzilla/blob/86fb87f/showdependencygraph.cgi#L200. Apparently, on landfill this causes it to escape the current DOT statement and inject additional commands. I cannot reproduce it in my own setup so I didn't do proper investigation here. I know that turning "malicious" into an absolute URL or adding XSS payload to it won't work but I don't really know why. I hope that somebody else can explain what exactly is happening there and whether this issue is exploitable.

Comment 1

[Frédéric Buclin]

Attachment: patch, v1

Unescaped newlines cause labels to be split into several lines in the .map file, which allows to inject the code you want. All you can do is to corrupt/alter the generated PNG image, so I don't think there is a security vulnerability here. But it's still bad to be able to inject your code.

So we escape newlines to be rendered as \\n, which are then correctly interpreted by dot. Now the .map file is correctly formatted (no unexpected linebreaks).

Comment 2

[Wladimir Palant]

After looking through http://www.graphviz.org/content/attrs, the DOT syntax itself doesn't seem to allow doing much damage. The biggest issue should be the various path parameters, the danger is that some confidential files will make it into the image. The risk seems limited however, in my tests non-image files were rejected.

Other than that the danger is mostly XSS due to Buzilla trusting webdot output. If one makes it produce a malicious URL Bugzilla will not escape it.

Unfortunately, https://landfill.bugzilla.org/bugzilla-tip/ is down. My own testing produces entirely different results, unescaped newlines don't break out of the label value. I can only imagine that this server is running some very old webdot version.

Comment 3

[David Lawrence [:dkl]]

Comment on attachment 8726860 [details] [diff] [review]
patch, v1

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

5.0 and 4.4 are not affected by the summary wrapping issue which id only in trunk. That were introduced by
http://git.mozilla.org/?p=bugzilla/bugzilla.git;a=commitdiff;h=2993c6a3 [github]

The newline escapes is still a good idea. r=dkl

Comment 4

[Frédéric Buclin]

You cannot inject newlines in bug summaries, because all bug summaries are cleaned up before being inserted into the DB. As 4.4 is not affected and is restricted to security fixes only, there is no need to commit it there.

Comment 5

[Frédéric Buclin]

For 5.0, this is really a "just in case" scenario as this shouldn't be possible to trigger.

To ssh://[e-mail]/bugzilla/bugzilla.git
   53d6ba4..27daf48  master -> master

To ssh://[e-mail]/bugzilla/bugzilla.git
   7621e45..1c5ecdf  5.0 -> 5.0

Comment 6

[Raymond Forbes[:rforbes]]

Could somebody please assign a security rating on this?

Comment 7

[Frédéric Buclin]

(In reply to Raymond Forbes[:rforbes] from comment #6)
> Could somebody please assign a security rating on this?

No release is affected, only master.

Comment 8

[Daniel Veditz [:dveditz]]

(In reply to Frédéric Buclin from comment #7)
> (In reply to Raymond Forbes[:rforbes] from comment #6)
> > Could somebody please assign a security rating on this?
> 
> No release is affected, only master.

Imagine Wladimir (or anyone else) never discovered this and then we released it in this state. What rating would it have in that case? Seems lowish?

Comment 9

[Frédéric Buclin]

(In reply to Daniel Veditz [:dveditz] from comment #8)
> Imagine Wladimir (or anyone else) never discovered this and then we released
> it in this state. What rating would it have in that case? Seems lowish?

Yes, seems lowish. You can corrupt the .dot file, but cannot inject anything AFAICT.

Comment 10

[Wladimir Palant]

As established in other bugs, Bugzilla trusts webdot output. If the .dot file can be manipulated successfully this can easily turn into XSS (still possible because the solution in bug 1253263 is incomplete in that respect).

Comment 11

[Wladimir Palant]

Note that "not released" should be read as "only BMO is affected."

Comment 12

[Daniel Veditz [:dveditz]]

Frédéric: does comment 10 change your rating?

Comment 13

[Frédéric Buclin]

(In reply to Daniel Veditz [:dveditz] from comment #12)
> Frédéric: does comment 10 change your rating?

Not really. As Wladimir said in comment 2, he didn't manage to abuse DOT in a usable way because he gets very different results compared to the very old version of DOT which was installed on landfill. So IMO the "this can *easily* turn into XSS" part of comment 10 is not true (note "easily").

Being able to corrupt the .dot file: definitely, yes. Doing XSS with it: no PoC so far.