276985 - XSS vulnerability in registry/who.cgi

Status: RESOLVED FIXED

(Webtools Graveyard :: Tinderbox, defect)

Description

[Michael Krax]

http://tinderbox.mozilla.org/registry/who.cgi?email=<script>alert
(document.cookie)</script>

http://tinderbox.mozilla.org/bonsai/cvsquery.cgi?
module=MozillaBranchTinderboxAll&branch=MOZILLA_1_7_BRANCH&date=explicit&mindate
=><script>alert(document.cookie)</script>

Tested with Internet Explorer 6 using WinXP SP2

Comment 1

[Michael Krax]

This XSS issue seems to be also part of tinderbox (sometimes the mozilla.org 
domain structure is a little confusing to me...)

http://axolotl.mozilla.org/graph/query.cgi?
testname=pageload&tbox=btek&autoscale=1&days=7&avg=1&showpoint=qwertz"><script>a
lert(document.cookie)</script>

Comment 2

[Daniel Veditz [:dveditz]]

I think the graph.cgi got fixed elsewhere, and cvsquery.cgi is bug 146244

Reassigning to justdave, and confirming because who.cgi is still borked. Not sure you could do much interesting with an XSS on tinderbox (or mecha, which includes bonsai and lxr). Are there admin pages on any of those services? If so you could craft an attack against someone with admin privs to do something interesting.

Ooh, yes, of course there are, "administrate tinderbox trees". That page requires a password for every submit though... there are probably others I'm not privy to.

Comment 3

[Reed Loden [:reed]]

Working testcase is at http://webtools.mozilla.org/registry/who.cgi?email=%3Cscript%3Ealert(document.cookie)%3C/script%3E

Comment 4

[Reed Loden [:reed]]

Attachment: patch - v1 (not tested)

I haven't tested this yet, but it looks somewhat sane. I also fixed some perl warnings while I was at it.

Comment 5

[Reed Loden [:reed]]

Attachment: patch - v2

I did some more clean-up to make the file more readable.

Comment 6

[Zach Lipton [:zach]]

Comment on attachment 249942 [details] [diff] [review]
patch - v2

I don't think there's any real way this can be exploited, but if we're going to fix it, the extra text and extra url fields in load_extra_data() should be quoted as well.

Comment 7

[Reed Loden [:reed]]

Attachment: patch - v3

Encode/quote everything and remove shell_encode().

Comment 8

[Reed Loden [:reed]]

Attachment: patch - v3.1

This is what I will check-in, as soon as the patch for 280464 is reviewed and ready.

Comment 9

[Dave Miller [:justdave]]

Attachment: Patch v4

OK, based on discussion on bug 280464 and on IRC, here's a do-over.

This completely replaces who.cgi.  Since the entire thing is basically a re-presentation of data passed in via the query string, the entire thing has been moved into a Template Toolkit template.  This makes it painless to keep track of what's been escaped and what hasn't, etc.

Comment 10

[Dave Miller [:justdave]]

Checking in who.cgi;
/cvsroot/mozilla/webtools/registry/who.cgi,v  <--  who.cgi
new revision: 1.12; previous revision: 1.11
done
RCS file: /cvsroot/mozilla/webtools/registry/who.html.tmpl,v
done
Checking in who.html.tmpl;
/cvsroot/mozilla/webtools/registry/who.html.tmpl,v  <--  who.html.tmpl
initial revision: 1.1
done

Comment 11

[Yvan Boily [:ygjb][:yvan]]

Adding keywords to bugs for metrics, no action required.  Sorry about bugmail spam.