503980 - show_bug.cgi doesn't properly escape <!-- inside bug summary

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Jeff Walden [:Waldo]]

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl&rev=1.155&mark=359#359

Passing through a comment-start sequence in that manner makes me uneasy, although I can't think of anything truly bad it actually does, absent template mods to make the template XHTML (and be sent with that MIME type).

Comment 1

[Max Kanat-Alexander]

I'm pretty sure that could be used as an XSS.

Comment 2

[Max Kanat-Alexander]

Well, maybe not an XSS, come to think of it (since we're probably still escaping other HTML tags? I dunnow) but we should investigate before removing the security restriction.

Comment 3

[Frédéric Buclin]

IMO, this isn't a bug at all. hideAliasAndSummary() will get a string with unescaped HTML entities in it, but JavaScript is not HMTL, and <!-- has no special meaning to it. This makes the bug INVALID to me, unless someone can prove there is any risk here.

Comment 4

[Frédéric Buclin]

(16:47:11) LpSolit: Jesse: is foo('<!-- blaba') a problem for JS?
(16:48:35) Jesse: inside a script tag? fine in HTML, i think, but not in XHTML
(16:48:56) Jesse: only thing you don't want in HTML is "</"
(16:50:20) LpSolit: Jesse: but in no way is this a security problem, right?
(16:53:15) Jesse: i don't think it is
(16:53:31) Jesse: unless some bugzilla users have attempted to use an XHTML mime type
(16:54:27) LpSolit: what would be the problem in that case?
(16:55:24) Jesse: you could probably insert stuff like <img src=foo onload=pwn /> in the same way

Comment 6

[Frédéric Buclin]

Attachment: patch, v1

Escaping "<" fixes the problem seen with Fx 3.6a1 with html5.enable = true, as described in bug 509373. I don't think we need to escape other HTML special characters.

Comment 7

[Max Kanat-Alexander]

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

Sure, looks fine to me.

We really should determine whether or not there's an actual security issue here, and if not, remove this from the security group.

Comment 8

[Max Kanat-Alexander]

Okay, not a security issue, after investigation.

Comment 9

[Frédéric Buclin]

tip:

Checking in Bugzilla/Template.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template.pm,v  <--  Template.pm
new revision: 1.104; previous revision: 1.103
done

3.4.1:

Checking in Bugzilla/Template.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template.pm,v  <--  Template.pm
new revision: 1.99.2.2; previous revision: 1.99.2.1
done