477477 - XSS vulnerability with pushloghtml's 'startdate' and 'enddate' params

Status: VERIFIED FIXED

(Developer Services :: Mercurial: hg.mozilla.org, defect)

Description

[Masahiro YAMADA]

mozilla-central's  query form has XSS vuluneravility.
Parameter enddate value is not HTML escaped.
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=48+hours+ago&enddate=now%22%20onmouseover=%22alert%28location.href%29

Comment 1

[Masahiro YAMADA]

startdate parameter has same problem.
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=48+hours+ago%22%20onmouseover=%22alert(location.href)

Comment 2

[Reed Loden [:reed]]

Attachment: patch - v1 (untested)

Escape the 'startdate' and 'enddate' values.

Comment 3

[Reed Loden [:reed]]

Comment on attachment 361147 [details] [diff] [review]
patch - v1 (untested)

ted or djc, please review?

Comment 4

[Dirkjan Ochtman (:djc)]

Comment on attachment 361147 [details] [diff] [review]
patch - v1 (untested)

Seems fine to me.

Comment 5

[Reed Loden [:reed]]

http://hg.mozilla.org/hg_templates/rev/244027b2b6dd

Pushing live now.

Comment 6

[Reed Loden [:reed]]

The example URLs now return 500 ISEs for the bad URLs, but normal stuff still seems to work. Is that expected?

Comment 7

[Reed Loden [:reed]]

Attachment: craziness setting in at a young age

Oh, looks like we need to import escape from mercurial.templatefilters.

Comment 8

[Reed Loden [:reed]]

Comment on attachment 361228 [details] [diff] [review]
craziness setting in at a young age

n/m, ignore me.

Comment 9

[Dirkjan Ochtman (:djc)]

So have the 500 ISE's been fixed now? What was the problem with that?

Comment 10

[Masahiro YAMADA]

I found some trivial problem on 500IES's message.
(root@localhost should be correct server operator's mail address)
I filed it as bug 477563.

Comment 11

[Reed Loden [:reed]]

mod_wsgi (pid=12696): Exception occurred processing WSGI script '/repo/hg/webroot_wsgi/hgwebdir.wsgi'.
Traceback (most recent call last):
  File "/usr/lib/python2.4/site-packages/mercurial/hgweb/request.py", line 100, in run_wsgi
    return application(env, respond)
  File "/usr/lib/python2.4/site-packages/mercurial/hgweb/hgwebdir_mod.py", line 74, in __call__
    self.run_wsgi(req)
  File "/usr/lib/python2.4/site-packages/mercurial/hgweb/hgwebdir_mod.py", line 121, in run_wsgi
    hgweb(repo).run_wsgi(req)
  File "/usr/lib/python2.4/site-packages/mercurial/hgweb/hgweb_mod.py", line 255, in run_wsgi
    req.write(content)
  File "/usr/lib/python2.4/site-packages/mercurial/hgweb/request.py", line 59, in write
    for part in thing:
  File "/usr/lib/python2.4/site-packages/mercurial/templater.py", line 126, in __call__
    v = self.filters[f](v)
  File "/usr/lib/python2.4/site-packages/mercurial/templatefilters.py", line 132, in <lambda>
    "escape": lambda x: cgi.escape(x, True),
  File "/usr/lib/python2.4/cgi.py", line 1039, in escape
    s = s.replace("&", "&amp;") # Must be done first!
AttributeError: 'list' object has no attribute 'replace'

Comment 12

[Reed Loden [:reed]]

(In reply to comment #9)
> So have the 500 ISE's been fixed now? What was the problem with that?

See comment #11. Any help would be appreciated! :)

Comment 13

[Dirkjan Ochtman (:djc)]

This should do the job:

diff --git a/pushlog-feed.py b/pushlog-feed.py
--- a/pushlog-feed.py
+++ b/pushlog-feed.py
@@ -402,8 +402,8 @@
                 rev=0,
                 entries=lambda **x: changelist(limit=0,**x),
                 latestentry=lambda **x: changelist(limit=1,**x),
-                startdate='startdate' in req.form and req.form['startdate'] or '1 week ago',
-                enddate='enddate' in req.form and req.form['enddate'] or 'now',
+                startdate='startdate' in req.form and req.form['startdate'][0] or '1 week ago',
+                enddate='enddate' in req.form and req.form['enddate'][0] or 'now',
                 querydescription=query.description(),
                 archives=web.archivelist("tip"))

Comment 14

[Reed Loden [:reed]]

Attachment: djc's fix for pushlog-feed.py's treatment of form members

That does indeed work. Here's the patch and my r= for it successfully working. I'll push this to the repo shortly.

Comment 15

[Reed Loden [:reed]]

http://hg.mozilla.org/users/bsmedberg_mozilla.com/hgpoller/rev/4087fe690caf

Comment 16

[Masahiro YAMADA]

I tested testcase in comment #0 and comment #1, and both of them are correctly HTML escaped.
--> VERIFIED.

Comment 17

[(not currently active) Ted Mielczarek]

I think 'critical' is a little overstated. hg.mozilla.org has no cookies and is read-only. AFAICT, there was no actual risk here.

Comment 18

[Masahiro YAMADA]

Changed importance from critical to major, baucase of comment #17.

Comment 19

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

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

Comment 20

[Gregory Szorc [:gps]]

https://hg.mozilla.org/hgcustom/version-control-tools/rev/8ef968b96556

Comment 21

[Gregory Szorc [:gps]]

https://hg.mozilla.org/hgcustom/version-control-tools/rev/0de933d5a48c