If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Slugify tags ("<script>alert('xss')</script>" should be invalid)

VERIFIED FIXED in 5.12.6

Status

addons.mozilla.org Graveyard
Developer Pages
VERIFIED FIXED
7 years ago
2 years ago

People

(Reporter: krupa, Assigned: andym)

Tracking

({wsec-xss})

5.12.6
wsec-xss

Details

(Whiteboard: [Edit page][infrasec-qa:input], URL)

(Reporter)

Description

7 years ago
steps to reproduce:
1. Go to the /edit page for any add-on
2. Click on "edit" for Basic Information
3. Add <script>alert('xss')</script> as a tag
4. Save changes
5. Load https://addons.allizom.org/z/en-US/developers/addons

expected behavior:
That tag should not be accepted or at least, get slugified. 

actual behavior:
All developers associated with that add-on will not be able to access their dev-tools dashboard.

traceback details:

 File "/usr/lib/python2.6/site-packages/jinja2/environment.py", line 891, in render
   return self.environment.handle_exception(exc_info, True)

 File "/data/amo_python/www/preview/zamboni/apps/devhub/templates/devhub/addons/dashboard.html", line 1, in top-level template code
   {% extends "devhub/base.html" %}

 File "/data/amo_python/www/preview/zamboni/apps/devhub/templates/devhub/base.html", line 1, in top-level template code
   {% extends "base.html" %}

 File "/data/amo_python/www/preview/zamboni/templates/base.html", line 89, in top-level template code
   {% block main_content %}

 File "/data/amo_python/www/preview/zamboni/templates/base.html", line 94, in block "main_content"
   {% block content %}{% endblock %}

 File "/data/amo_python/www/preview/zamboni/apps/devhub/templates/devhub/addons/dashboard.html", line 71, in block "content"
   {{ item|safe }}

 File "/usr/lib/python2.6/site-packages/jinja2/filters.py", line 623, in do_mark_safe
   return Markup(value)

 File "/usr/lib/python2.6/site-packages/markupsafe/__init__.py", line 71, in __new__
   return unicode.__new__(cls, base)

 File "/data/amo_python/www/preview/zamboni/apps/devhub/models.py", line 214, in __unicode__
   return self.to_string()

 File "/data/amo_python/www/preview/zamboni/apps/devhub/models.py", line 203, in to_string
   tag = u'<a href="%s">%s</a>' % (arg.get_url_path(),

 File "/data/amo_python/www/preview/zamboni/apps/tags/models.py", line 34, in get_url_path
   return reverse('tags.detail', args=[self.tag_text])

 File "/data/amo_python/www/preview/zamboni/apps/amo/urlresolvers.py", line 58, in reverse
   url = django_reverse(viewname, urlconf, args, kwargs, prefix, current_app)

 File "/data/amo_python/www/preview/zamboni/vendor/src/django/django/core/urlresolvers.py", line 390, in reverse
   *args, **kwargs)))

 File "/data/amo_python/www/preview/zamboni/vendor/src/django/django/core/urlresolvers.py", line 336, in reverse
   "arguments '%s' not found." % (lookup_view_s, args, kwargs))

NoReverseMatch: Reverse for 'tags.detail' with arguments '(u"<script>alert('xss')</script>",)' and keyword arguments '{}' not found.
This is not an xss; the page doesn't even load.  This is an input validation issue.
Group: client-services-security
Severity: major → normal
Summary: NoReverseMatch: Reverse for 'tags.detail' with arguments '(u"<script>alert('xss')</script>",)' and keyword arguments '{}' not found → Slugify tags ("<script>alert('xss')</script>" should be invalid)
Whiteboard: [Edit page][infrasec-qa:xss] → [Edit page]
Target Milestone: 5.12.5 → 5.12.6
Whiteboard: [Edit page] → [Edit page][infrasec-qa:input]
We should come up with a list of allowed characters for tags-- slugifying is too strict (at the very least, we need spaces), however we're probably too liberal in what we allow.
(Assignee)

Updated

7 years ago
Assignee: nobody → amckay
We should allow the same thing we allow collection slugs (which includes unicode).  I don't know if there is a constant for it like SLUG_OK though.
(Assignee)

Comment 4

7 years ago
Slugify looks good to me. Tags shouldn't really have spaces imho. 

Presumably will need to write a migration to clean up and conflate tags too.
(Assignee)

Comment 5

7 years ago
Migration to clean up and conflate is a new ticket:

https://bugzilla.mozilla.org/show_bug.cgi?id=621811
(Assignee)

Comment 6

7 years ago
Tags slugified. Spaces within a tag preserved. Spaces on either end stripped. Tags lowercased.

https://github.com/jbalogh/zamboni/commit/9287ad449d62bf918466298f8c5e6af0c9003dc3
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Reporter)

Comment 7

7 years ago
verified @ https://addons.allizom.org/z/en-US/developers/addon/amazonassist-test-addon3011/edit
Status: RESOLVED → VERIFIED
Adding keywords to bugs for metrics, no action required.  Sorry about bugmail spam.
Keywords: wsec-xss
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.