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

VERIFIED FIXED

Status

Developer Services
Mercurial: hg.mozilla.org
--
major
VERIFIED FIXED
8 years ago
3 years ago

People

(Reporter: Masahiro YAMADA, Assigned: reed)

Tracking

({wsec-xss})

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

8 years ago
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
(Reporter)

Updated

8 years ago
Summary: Mozilla central XSS vulneravility → Mozilla central XSS vulnerability
(Assignee)

Updated

8 years ago
Group: mozilla-confidential → webtools-security
Summary: Mozilla central XSS vulnerability → XSS vulnerability with pushloghtml's 'enddate' param
(Reporter)

Comment 1

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

Comment 2

8 years ago
Created attachment 361147 [details] [diff] [review]
patch - v1 (untested)

Escape the 'startdate' and 'enddate' values.
Assignee: nobody → reed
Status: NEW → ASSIGNED
Attachment #361147 - Flags: review?
(Assignee)

Updated

8 years ago
Summary: XSS vulnerability with pushloghtml's 'enddate' param → XSS vulnerability with pushloghtml's 'startdate' and 'enddate' params
(Assignee)

Comment 3

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

ted or djc, please review?
Attachment #361147 - Flags: review?(ted.mielczarek)
Attachment #361147 - Flags: review?(dirkjan)
Attachment #361147 - Flags: review?

Updated

8 years ago
Attachment #361147 - Flags: review?(dirkjan) → review+
Comment on attachment 361147 [details] [diff] [review]
patch - v1 (untested)

Seems fine to me.
(Assignee)

Updated

8 years ago
Attachment #361147 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 5

8 years ago
http://hg.mozilla.org/hg_templates/rev/244027b2b6dd

Pushing live now.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Updated

8 years ago
Group: webtools-security
(Assignee)

Comment 6

8 years ago
The example URLs now return 500 ISEs for the bad URLs, but normal stuff still seems to work. Is that expected?
(Assignee)

Comment 7

8 years ago
Created attachment 361228 [details] [diff] [review]
craziness setting in at a young age

Oh, looks like we need to import escape from mercurial.templatefilters.
Attachment #361228 - Flags: review?(dirkjan)
(Assignee)

Comment 8

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

n/m, ignore me.
Attachment #361228 - Attachment is obsolete: true
Attachment #361228 - Flags: review?(dirkjan)
So have the 500 ISE's been fixed now? What was the problem with that?
(Assignee)

Updated

8 years ago
Attachment #361228 - Attachment description: Import 'escape' from mercurial.templatefilters - v1 → craziness setting in at a young age
(Reporter)

Comment 10

8 years ago
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.
(Assignee)

Comment 11

8 years ago
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'
(Assignee)

Comment 12

8 years ago
(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! :)
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"))
(Assignee)

Comment 14

8 years ago
Created attachment 361231 [details] [diff] [review]
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.
Attachment #361231 - Flags: review+
(Assignee)

Comment 15

8 years ago
http://hg.mozilla.org/users/bsmedberg_mozilla.com/hgpoller/rev/4087fe690caf
(Reporter)

Comment 16

8 years ago
I tested testcase in comment #0 and comment #1, and both of them are correctly HTML escaped.
--> VERIFIED.
Status: RESOLVED → VERIFIED
I think 'critical' is a little overstated. hg.mozilla.org has no cookies and is read-only. AFAICT, there was no actual risk here.
(Reporter)

Comment 18

8 years ago
Changed importance from critical to major, baucase of comment #17.
Severity: critical → major
Adding keywords to bugs for metrics, no action required.  Sorry about bugmail spam.
Keywords: wsec-xss
Product: mozilla.org → Release Engineering
https://hg.mozilla.org/hgcustom/version-control-tools/rev/8ef968b96556
https://hg.mozilla.org/hgcustom/version-control-tools/rev/0de933d5a48c
Product: Release Engineering → Developer Services
You need to log in before you can comment on or make changes to this bug.