Closed Bug 272620 Opened 16 years ago Closed 16 years ago
XSS vulnerability in internal error messages
Assignee: mozilla.webmaster → justdave
Group: security → webtools-security
Component: firstname.lastname@example.org → Bugzilla-General
Product: mozilla.org → Bugzilla
QA Contact: daniel.bugmail → mattyt-bugzilla
Version: other → 2.10
Here's the link, unbroken, for reference: https://bugzilla.mozilla.org/attachment.cgi?id=&action=force_internal_error<script>alert(document.cookie)</script> And Landfill links for testing: http://landfill.bugzilla.org/bugzilla-tip/attachment.cgi?id=&action=force_internal_error<script>alert(document.cookie)</script> http://landfill.bugzilla.org/bugzilla-2.18-branch/attachment.cgi?id=&action=force_internal_error<script>alert(document.cookie)</script> http://landfill.bugzilla.org/bugzilla-2.16-branch/attachment.cgi?id=&action=force_internal_error<script>alert(document.cookie)</script>
OS: Windows XP → All
Hardware: PC → All
Target Milestone: --- → Bugzilla 2.16
Version: 2.10 → 2.17.6
Good catch, Michael. This was, I believe, my mistake. :-( Gerv
Option 3 is to take a short cut and use escape(), which means the URL will be mangled but also both safe and easily decodable. E.g. http://bugzilla.mozilla.org/enter_bug.cgi?<script>alert('foo');</script> -> http%3A%2F%2Fbugzilla.mozilla.org%2Fenter_bug.cgi%3F%3Cscript%3Ealert('foo')%3B%3C%2Fscript%3E Gerv
Summary: XSS Vulnerability in Internal Error Messages → XSS vulnerability in internal error messages
Assignee: justdave → vladd
Status: NEW → ASSIGNED
Comment on attachment 168286 [details] [diff] [review] v1 - option C escape() is not UTF-8 safe and is deprecated for that reason. Use encodeURI() instead.
Attachment #168286 - Flags: review?(gerv) → review-
Comment on attachment 168288 [details] [diff] [review] v1 - option A If we go this route, we need to also ask the user to include the URL from the URL bar in their email.
Attachment #168288 - Flags: review?(gerv) → review-
encodeURI requires IE 5.5 (http://msdn.microsoft.com/library/default.asp?url=/library/en-us/script56/html/js56jsmthfencodeuri.asp) and I'm fairly sure it's not supported in Netscape 4. Given that it's not absolutely totally vital that it's lossless, and encode() works everywhere and correctly for 99.9% of cases, isn't that the right choice for the 2.18 branch? Gerv
escape() is not UTF-8 safe and encodeURI requires IE 5.5. Why not escaping < and > only using their equivalent %3C and %3E respectively? I think there is no need to escape the whole URL.
If we did that, we would just use < and > and &, I think. But yes, that's another option. Gerv
I found 3 files having document.location in a JS script: Error.pm line 121 code-error.html.tmpl line 259 quicksearch.js line 56 I think we should take care of all of them. I tried a simple document.location.replace(/</g,"%3C").replace(/>/g,"%3E"); but this does not work as "<" in the URL seems to induce some strange behavior. :(
On next tuesday this vulnerability will be made public with 170 other flaws as part of a general paper on cross-site scripting. There will be no detailed description, only a proof-of-concept URL. Just to let you know beforehand.
This has been allocated CVE name CAN-2004-1061. http://cve.mitre.org/cgi-bin/cvename.cgi?name=CAN-2004-1061 This is currently not visible since the flaw is not yet public. Please include this link in the future security announcement.
(In reply to comment #13) > I tried a simple > document.location.replace(/</g,"%3C").replace(/>/g,"%3E"); > but this does not work as "<" in the URL seems to induce some strange behavior. :( What kind of strange behavior?
The problem is that he did document.location.replace(), which is a function which redirects the browser, not document.location.href.replace(), which will substitute characters in the URL ;-) Patch coming right up. Gerv
pfff... only missing .href ! :( I thought I could fix my first security bug... Well, later maybe... ;)
This patches the instances in code-error.html.tmpl and Error.pm. The use of document.location.href in quicksearch.js doesn't print the location to the page and so is not a problem. I've taken the approach of using HTML entities so the page still renders the exact URL that the user used. On Linux, Konqueror is a good browser for testing this fix, because like IE it doesn't enforce correct URL encoding. Gerv
vladd: I just realised I took this bug without thinking. Please don't be offended :-) Gerv
does this patch apply cleanly to all three branches? /me doubts it works in 2.16, since 2.16 didn't have Error.pm
Untested backport to 2.16.7, which hopefully applies to the branch. Different placement of </p> (on next line), and slightly different indentation (document.location.href should be on the same level as the quotes, I'd say).
Attachment #168868 - Flags: review?
Comment on attachment 168868 [details] [diff] [review] 2.16 backport (use patch -p1) r=gerv by inspection. Gerv
Attachment #168868 - Flags: review? → review+
Whiteboard: patch awaiting review → [ready for 2.16.8] [ready for 2.18] [ready for 2.20rc1]
Despite having a security fix available on 2004-12-16 for all 3 branches, we failed to do a release between then and the time when the security was made public, which was 7 days later. I modified that status whiteboard of this bug in order to indicate that someone needs to do an analysis of what happened and where we failed to release a security patch to the public in due time. If nobody else volunteers I'll investigate the issue when I'll find some free time.
Whiteboard: [ready for 2.16.8] [ready for 2.18] [ready for 2.20rc1] → [ready for 2.16.8] [ready for 2.18] [ready for 2.20rc1] [BZ-security management analysis needed]
Comment on attachment 168807 [details] [diff] [review] Patch for 2.18 and 2.19.1 (use patch -p0) Removing unneeded review.
Indeed. We suck. However, XSS is not arbitrary database manipulation, and if someone does get XSSed using this hole, they'll get an error message from Bugzilla they didn't expect, along with a big red suspicious-looking URL. I say we just put the afterburners on and release 2.18 proper, and 2.16.whatever, ASAP. Gerv
(In reply to comment #27) > However, XSS is not arbitrary database manipulation, and if > someone does get XSSed using this hole, they'll get an error message from > Bugzilla they didn't expect, along with a big red suspicious-looking URL. XSS can't be used to manipulate data server side (only indirect by e.g. stealing a session cookie width administrative rights) but the client output is totally under the attackers control: https://bugzilla.mozilla.org/attachment.cgi? id=&action=force_internal_error<script>s=String.fromCharCode(32);document.write ("<div"+s+"style='position:absolute;top:85px;left:0px;padding:30px;background- color:white;width:101%;height:100%;text-align:center;font-size:30px;font- family:arial;'>MOZILLA.ORG"+s+"GIVES"+s+"UP<br><br>USE"+s+"INTERNET"+s+"EXPLORER "+s+"INSTEAD</div>")</script> You could also include a way longer script from another webserver to fake whatever you want.
Michael: I am aware of what's possible with XSS. But you must admit that this is in a different league to e.g. "any Bugzilla user can trash the database". Please don't think I'm belittling this vulnerability. I wrote the code in our test suite which tests our templates for possible XSS vulnerabilities. I am taking this seriously. I hope "ASAP" will mean "in the next couple of days". I've already triaged the 2.18 buglist and reviewed all the patches I can. We have one outstanding bug of any size - bug 108870 - and I'm pushing to get that fixed. Gerv
(In reply to comment #29) > But you must admit that this is > in a different league to e.g. "any Bugzilla user can trash the database". Agreed, it's a minor security issue. Wasn't meant to offend you ;) I just get a lot of emails doubting that XSS is a securtiy issue at all these days...
(In reply to comment #25) > Despite having a security fix available on 2004-12-16 for all 3 branches, we > failed to do a release between then and the time when the security was made > public, which was 7 days later. What happened is pretty simple. We're SO FREAKING CLOSE to releasing a final 2.18 that releasing a 2.18rc4 just seemed stupid. In hindsight, since nobody had time to patch/review the last 2.18 blockers during that time period, we should have done it anyway, but we didn't. I did propose just making the patch available with the advisory instead of doing a full-scale release, but I got poopooed on that idea in IRC. That doesn't excuse it though, I should have ran with it anyway. Removing the security flag since this is now publicly disclosed. You may proceed with checkins, whether we're ready to release or not. The only reason to delay checkins is to prevent premature disclosure, and it's too late for that.
Whiteboard: [ready for 2.16.8] [ready for 2.18] [ready for 2.20rc1] [BZ-security management analysis needed] → [ready for 2.16.8] [ready for 2.18] [ready for 2.20rc1]
Checked in on trunk, 2.18 branch and 2.16 branch. Checking in template/en/default/global/code-error.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v <-- code-error.html.tmpl new revision: 1.43; previous revision: 1.42 done Checking in Bugzilla/Error.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Error.pm,v <-- Error.pm new revision: 1.9; previous revision: 1.8 done Checking in template/en/default/global/code-error.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v <-- code-error.html.tmpl new revision: 220.127.116.11; previous revision: 18.104.22.168 done Checking in Bugzilla/Error.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Error.pm,v <-- Error.pm new revision: 22.214.171.124; previous revision: 1.4 done Checking in CGI.pl; /cvsroot/mozilla/webtools/bugzilla/CGI.pl,v <-- CGI.pl new revision: 126.96.36.199; previous revision: 188.8.131.52 done Checking in template/en/default/global/code-error.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v <-- code-error.html.tmpl new revision: 184.108.40.206; previous revision: 220.127.116.11 done Gerv
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Also of note is that I had a hard drive failure on my primary workstation the day before this was disclosed, and had limited access to email for 3 days before I got the workstation fixed. Life sucks sometimes.
Attachment #168868 - Attachment description: 2.16 backport → 2.16 backport (use patch -p1)
Attachment #168807 - Attachment description: Patch v.1 - Option D ;-) → Patch for 2.18 and 2.19.1 (use patch -p0)
You need to log in before you can comment on or make changes to this bug.