From c844bd692894353c60b320005b804970605e910f Mon Sep 17 00:00:00 2001 From: Julie Pichon Date: Thu, 22 May 2014 16:45:03 +0100 Subject: [PATCH] Fix multiple Cross-Site Scripting (XSS) vulnerabilities * Ensure user emails are properly escaped User emails in the Users and Groups panel are being passed through the urlize filter to transform them into clickable links. However, urlize expects input to be already escaped and safe. We should make sure to escape the strings first as email addresses are not validated and can contain any type of string. Closes-Bug: #1320235 * Ensure network names are properly escaped in the Launch Instance menu Closes-Bug: #1322197 * Escape the URLs generated for the Horizon tables When generating the Horizon tables, there was an assumption that only the anchor text needed to be escaped. However some URLs are generated based on user-provided data and should be escaped as well. Also escape the link attributes for good measure. * Use 'reverse' to generate the Resource URLs in the stacks tables Closes-Bug: #1308727 Conflicts: horizon/tables/base.py openstack_dashboard/dashboards/admin/users/tables.py Change-Id: Ic8a92e69f66c2d265a802f350e30f091181aa42e --- horizon/static/horizon/js/horizon.instances.js | 9 ++++++++- horizon/tables/base.py | 4 +++- openstack_dashboard/dashboards/admin/groups/tables.py | 3 ++- openstack_dashboard/dashboards/admin/users/tables.py | 3 ++- .../dashboards/project/stacks/tables.py | 10 ++++++++-- openstack_dashboard/dashboards/project/stacks/tabs.py | 6 ++++++ 6 files changed, 29 insertions(+), 6 deletions(-) diff --git a/horizon/static/horizon/js/horizon.instances.js b/horizon/static/horizon/js/horizon.instances.js index c90118098f1..c6ff3233c9f 100644 --- a/horizon/static/horizon/js/horizon.instances.js +++ b/horizon/static/horizon/js/horizon.instances.js @@ -51,8 +51,15 @@ horizon.instances = { $(this.get_network_element("")).each(function(){ var $this = $(this); var $input = $this.children("input"); + var name = $this.text().replace(/^\s+/,"") + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, ''') + .replace(/\//g, '/'); var network_property = { - name:$this.text().replace(/^\s+/,""), + name:name, id:$input.attr("id"), value:$input.attr("value") }; diff --git a/horizon/tables/base.py b/horizon/tables/base.py index adc284c9ea2..9011b77d305 100644 --- a/horizon/tables/base.py +++ b/horizon/tables/base.py @@ -585,7 +585,9 @@ def value(self): link_classes = ' '.join(self.column.link_classes) # Escape the data inside while allowing our HTML to render data = mark_safe('%s' % - (self.url, link_classes, escape(data))) + (escape(self.url), + escape(link_classes), + escape(data))) return data @property diff --git a/openstack_dashboard/dashboards/admin/groups/tables.py b/openstack_dashboard/dashboards/admin/groups/tables.py index bce8f5008b7..ff8103bebf0 100644 --- a/openstack_dashboard/dashboards/admin/groups/tables.py +++ b/openstack_dashboard/dashboards/admin/groups/tables.py @@ -161,7 +161,8 @@ def get_link_url(self, datum=None): class UsersTable(tables.DataTable): name = tables.Column('name', verbose_name=_('User Name')) email = tables.Column('email', verbose_name=_('Email'), - filters=[defaultfilters.urlize]) + filters=[defaultfilters.escape, + defaultfilters.urlize]) id = tables.Column('id', verbose_name=_('User ID')) enabled = tables.Column('enabled', verbose_name=_('Enabled'), status=True, diff --git a/openstack_dashboard/dashboards/admin/users/tables.py b/openstack_dashboard/dashboards/admin/users/tables.py index d47d68d22f0..c0b0ea5a243 100644 --- a/openstack_dashboard/dashboards/admin/users/tables.py +++ b/openstack_dashboard/dashboards/admin/users/tables.py @@ -117,7 +117,8 @@ class UsersTable(tables.DataTable): ) name = tables.Column('name', verbose_name=_('User Name')) email = tables.Column('email', verbose_name=_('Email'), - filters=[defaultfilters.urlize]) + filters=[defaultfilters.escape, + defaultfilters.urlize]) # Default tenant is not returned from Keystone currently. #default_tenant = tables.Column('default_tenant', # verbose_name=_('Default Project')) diff --git a/openstack_dashboard/dashboards/project/stacks/tables.py b/openstack_dashboard/dashboards/project/stacks/tables.py index f0bc731ead0..822726be194 100644 --- a/openstack_dashboard/dashboards/project/stacks/tables.py +++ b/openstack_dashboard/dashboards/project/stacks/tables.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. +from django.core import urlresolvers from django.http import Http404 # noqa from django.template.defaultfilters import timesince # noqa from django.template.defaultfilters import title # noqa @@ -94,11 +95,16 @@ class Meta: row_actions = (DeleteStack, ) +def get_resource_url(obj): + return urlresolvers.reverse('horizon:project:stacks:resource', + args=(obj.stack_id, obj.resource_name)) + + class EventsTable(tables.DataTable): logical_resource = tables.Column('resource_name', verbose_name=_("Stack Resource"), - link=lambda d: d.resource_name,) + link=get_resource_url) physical_resource = tables.Column('physical_resource_id', verbose_name=_("Resource"), link=mappings.resource_to_url) @@ -142,7 +148,7 @@ class ResourcesTable(tables.DataTable): logical_resource = tables.Column('resource_name', verbose_name=_("Stack Resource"), - link=lambda d: d.resource_name) + link=get_resource_url) physical_resource = tables.Column('physical_resource_id', verbose_name=_("Resource"), link=mappings.resource_to_url) diff --git a/openstack_dashboard/dashboards/project/stacks/tabs.py b/openstack_dashboard/dashboards/project/stacks/tabs.py index 15ef8338806..b5886f3f49c 100644 --- a/openstack_dashboard/dashboards/project/stacks/tabs.py +++ b/openstack_dashboard/dashboards/project/stacks/tabs.py @@ -75,6 +75,9 @@ def get_context_data(self, request): stack_identifier = '%s/%s' % (stack.stack_name, stack.id) events = api.heat.events_list(self.request, stack_identifier) LOG.debug('got events %s' % events) + # The stack id is needed to generate the resource URL. + for event in events: + event.stack_id = stack.id except Exception: events = [] messages.error(request, _( @@ -95,6 +98,9 @@ def get_context_data(self, request): stack_identifier = '%s/%s' % (stack.stack_name, stack.id) resources = api.heat.resources_list(self.request, stack_identifier) LOG.debug('got resources %s' % resources) + # The stack id is needed to generate the resource URL. + for r in resources: + r.stack_id = stack.id except Exception: resources = [] messages.error(request, _(