[security] prevent XSS on FAB list views (#1125)

* [security] prevent XSS on FAB list views

* addressing comments
This commit is contained in:
Maxime Beauchemin 2016-09-16 16:25:42 -07:00 committed by GitHub
parent e8f1baba43
commit b62d7e3e8e
3 changed files with 20 additions and 20 deletions

View File

@ -22,7 +22,7 @@ from sqlalchemy.engine.url import make_url
import sqlparse import sqlparse
from dateutil.parser import parse 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 import Model
from flask_appbuilder.models.mixins import AuditMixin from flask_appbuilder.models.mixins import AuditMixin
from flask_appbuilder.models.decorators import renders from flask_appbuilder.models.decorators import renders
@ -102,12 +102,13 @@ class AuditMixinNullable(AuditMixin):
@renders('changed_on') @renders('changed_on')
def changed_on_(self): 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') @renders('changed_on')
def modified(self): def modified(self):
s = humanize.naturaltime(datetime.now() - self.changed_on) 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 @property
def icons(self): def icons(self):
@ -252,8 +253,8 @@ class Slice(Model, AuditMixinNullable):
@property @property
def slice_link(self): def slice_link(self):
url = self.slice_url url = self.slice_url
return '<a href="{url}">{obj.slice_name}</a>'.format( name = escape(self.slice_name)
url=url, obj=self) return Markup('<a href="{url}">{name}</a>'.format(**locals()))
def get_viz(self, url_params_multidict=None): def get_viz(self, url_params_multidict=None):
"""Creates :py:class:viz.BaseViz object from the url_params_multidict. """Creates :py:class:viz.BaseViz object from the url_params_multidict.
@ -349,7 +350,9 @@ class Dashboard(Model, AuditMixinNullable):
return metadata.reflect() return metadata.reflect()
def dashboard_link(self): 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 @property
def json_data(self): def json_data(self):
@ -684,7 +687,9 @@ class SqlaTable(Model, Queryable, AuditMixinNullable):
@property @property
def link(self): 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 @property
def perm(self): def perm(self):
@ -728,10 +733,6 @@ class SqlaTable(Model, Queryable, AuditMixinNullable):
def name(self): def name(self):
return self.table_name 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 @property
def metrics_combo(self): def metrics_combo(self):
return sorted( return sorted(
@ -1231,9 +1232,8 @@ class DruidDatasource(Model, AuditMixinNullable, Queryable):
@property @property
def link(self): def link(self):
return ( name = escape(self.datasource_name)
'<a href="{self.url}">' return Markup('<a href="{self.url}">{name}</a>').format(**locals())
'{self.datasource_name}</a>').format(**locals())
@property @property
def full_name(self): def full_name(self):
@ -1247,8 +1247,8 @@ class DruidDatasource(Model, AuditMixinNullable, Queryable):
@renders('datasource_name') @renders('datasource_name')
def datasource_link(self): def datasource_link(self):
url = "/caravel/explore/{obj.type}/{obj.id}/".format(obj=self) url = "/caravel/explore/{obj.type}/{obj.id}/".format(obj=self)
return '<a href="{url}">{obj.datasource_name}</a>'.format( name = escape(self.datasource_name)
url=url, obj=self) return Markup('<a href="{url}">{name}</a>'.format(**locals()))
def get_metric_obj(self, metric_name): def get_metric_obj(self, metric_name):
return [ return [

View File

@ -66,7 +66,7 @@
id="{{ '{}__{}'.format(pk, value) }}" id="{{ '{}__{}'.format(pk, value) }}"
data-checkbox-api-prefix="/caravel/checkbox/{{ modelview_name }}/{{ pk }}/{{ value }}/"> data-checkbox-api-prefix="/caravel/checkbox/{{ modelview_name }}/{{ pk }}/{{ value }}/">
{% else %} {% else %}
{{ item[value]|safe }} {{ item[value] }}
{% endif %} {% endif %}
</td> </td>
{% endfor %} {% endfor %}

View File

@ -535,10 +535,10 @@ appbuilder.add_view_no_menu(DatabaseTablesAsync)
class TableModelView(CaravelModelView, DeleteMixin): # noqa class TableModelView(CaravelModelView, DeleteMixin): # noqa
datamodel = SQLAInterface(models.SqlaTable) datamodel = SQLAInterface(models.SqlaTable)
list_columns = [ list_columns = [
'table_link', 'database', 'is_featured', 'link', 'database', 'is_featured',
'changed_by_', 'changed_on_'] 'changed_by_', 'changed_on_']
order_columns = [ order_columns = [
'table_link', 'database', 'is_featured', 'changed_on_'] 'link', 'database', 'is_featured', 'changed_on_']
add_columns = ['table_name', 'database', 'schema'] add_columns = ['table_name', 'database', 'schema']
edit_columns = [ edit_columns = [
'table_name', 'sql', 'is_featured', 'database', 'schema', 'table_name', 'sql', 'is_featured', 'database', 'schema',
@ -563,7 +563,7 @@ class TableModelView(CaravelModelView, DeleteMixin): # noqa
} }
base_filters = [['id', TableSlice, lambda: []]] base_filters = [['id', TableSlice, lambda: []]]
label_columns = { label_columns = {
'table_link': _("Table"), 'link': _("Table"),
'changed_by_': _("Changed By"), 'changed_by_': _("Changed By"),
'database': _("Database"), 'database': _("Database"),
'changed_on_': _("Last Changed"), 'changed_on_': _("Last Changed"),