Skip to content

Commit

Permalink
[security] prevent XSS on FAB list views (#1125)
Browse files Browse the repository at this point in the history
* [security] prevent XSS on FAB list views

* addressing comments
  • Loading branch information
mistercrunch committed Sep 16, 2016
1 parent e8f1bab commit b62d7e3
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 20 deletions.
32 changes: 16 additions & 16 deletions caravel/models.py
Expand Up @@ -22,7 +22,7 @@
import sqlparse
from dateutil.parser import parse

from flask import request, g
from flask import escape, g, Markup, request
from flask_appbuilder import Model
from flask_appbuilder.models.mixins import AuditMixin
from flask_appbuilder.models.decorators import renders
Expand Down Expand Up @@ -102,12 +102,13 @@ def changed_by_(self):

@renders('changed_on')
def changed_on_(self):
return '<span class="no-wrap">{}</span>'.format(self.changed_on)
return Markup(
'<span class="no-wrap">{}</span>'.format(self.changed_on))

@renders('changed_on')
def modified(self):
s = humanize.naturaltime(datetime.now() - self.changed_on)
return '<span class="no-wrap">{}</nobr>'.format(s)
return Markup('<span class="no-wrap">{}</span>'.format(s))

@property
def icons(self):
Expand Down Expand Up @@ -252,8 +253,8 @@ def edit_url(self):
@property
def slice_link(self):
url = self.slice_url
return '<a href="{url}">{obj.slice_name}</a>'.format(
url=url, obj=self)
name = escape(self.slice_name)
return Markup('<a href="{url}">{name}</a>'.format(**locals()))

def get_viz(self, url_params_multidict=None):
"""Creates :py:class:viz.BaseViz object from the url_params_multidict.
Expand Down Expand Up @@ -349,7 +350,9 @@ def sqla_metadata(self):
return metadata.reflect()

def dashboard_link(self):
return '<a href="{obj.url}">{obj.dashboard_title}</a>'.format(obj=self)
title = escape(self.dashboard_title)
return Markup(
'<a href="{self.url}">{title}</a>'.format(**locals()))

@property
def json_data(self):
Expand Down Expand Up @@ -684,7 +687,9 @@ def description_markeddown(self):

@property
def link(self):
return '<a href="{self.url}">{self.table_name}</a>'.format(**locals())
table_name = escape(self.table_name)
return Markup(
'<a href="{self.url}">{table_name}</a>'.format(**locals()))

@property
def perm(self):
Expand Down Expand Up @@ -728,10 +733,6 @@ def html(self):
def name(self):
return self.table_name

@renders('table_name')
def table_link(self):
return '<a href="{obj.explore_url}">{obj.table_name}</a>'.format(obj=self)

@property
def metrics_combo(self):
return sorted(
Expand Down Expand Up @@ -1231,9 +1232,8 @@ def perm(self):

@property
def link(self):
return (
'<a href="{self.url}">'
'{self.datasource_name}</a>').format(**locals())
name = escape(self.datasource_name)
return Markup('<a href="{self.url}">{name}</a>').format(**locals())

@property
def full_name(self):
Expand All @@ -1247,8 +1247,8 @@ def __repr__(self):
@renders('datasource_name')
def datasource_link(self):
url = "/caravel/explore/{obj.type}/{obj.id}/".format(obj=self)
return '<a href="{url}">{obj.datasource_name}</a>'.format(
url=url, obj=self)
name = escape(self.datasource_name)
return Markup('<a href="{url}">{name}</a>'.format(**locals()))

def get_metric_obj(self, metric_name):
return [
Expand Down
2 changes: 1 addition & 1 deletion caravel/templates/appbuilder/general/widgets/list.html
Expand Up @@ -66,7 +66,7 @@
id="{{ '{}__{}'.format(pk, value) }}"
data-checkbox-api-prefix="/caravel/checkbox/{{ modelview_name }}/{{ pk }}/{{ value }}/">
{% else %}
{{ item[value]|safe }}
{{ item[value] }}
{% endif %}
</td>
{% endfor %}
Expand Down
6 changes: 3 additions & 3 deletions caravel/views.py
Expand Up @@ -535,10 +535,10 @@ class DatabaseTablesAsync(DatabaseView):
class TableModelView(CaravelModelView, DeleteMixin): # noqa
datamodel = SQLAInterface(models.SqlaTable)
list_columns = [
'table_link', 'database', 'is_featured',
'link', 'database', 'is_featured',
'changed_by_', 'changed_on_']
order_columns = [
'table_link', 'database', 'is_featured', 'changed_on_']
'link', 'database', 'is_featured', 'changed_on_']
add_columns = ['table_name', 'database', 'schema']
edit_columns = [
'table_name', 'sql', 'is_featured', 'database', 'schema',
Expand All @@ -563,7 +563,7 @@ class TableModelView(CaravelModelView, DeleteMixin): # noqa
}
base_filters = [['id', TableSlice, lambda: []]]
label_columns = {
'table_link': _("Table"),
'link': _("Table"),
'changed_by_': _("Changed By"),
'database': _("Database"),
'changed_on_': _("Last Changed"),
Expand Down

0 comments on commit b62d7e3

Please sign in to comment.