Skip to content

Commit

Permalink
Fix multiple Cross-Site Scripting (XSS) vulnerabilities
Browse files Browse the repository at this point in the history
 * 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
  • Loading branch information
jpichon committed Jul 8, 2014
1 parent 19634d6 commit c844bd6
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 6 deletions.
9 changes: 8 additions & 1 deletion horizon/static/horizon/js/horizon.instances.js
Original file line number Diff line number Diff line change
Expand Up @@ -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, '&lt;')
.replace(/>/g, '&gt;')
.replace(/"/g, '&quot;')
.replace(/'/g, '&#x27;')
.replace(/\//g, '&#x2F;');
var network_property = {
name:$this.text().replace(/^\s+/,""),
name:name,
id:$input.attr("id"),
value:$input.attr("value")
};
Expand Down
4 changes: 3 additions & 1 deletion horizon/tables/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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('<a href="%s" class="%s">%s</a>' %
(self.url, link_classes, escape(data)))
(escape(self.url),
escape(link_classes),
escape(data)))
return data

@property
Expand Down
3 changes: 2 additions & 1 deletion openstack_dashboard/dashboards/admin/groups/tables.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion openstack_dashboard/dashboards/admin/users/tables.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'))
Expand Down
10 changes: 8 additions & 2 deletions openstack_dashboard/dashboards/project/stacks/tables.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions openstack_dashboard/dashboards/project/stacks/tabs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, _(
Expand All @@ -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, _(
Expand Down

0 comments on commit c844bd6

Please sign in to comment.