Fix 4 security vulnerabilities (#4390)

* Switched yaml.load to yaml.safe_load to prevent code execution via crafted yaml files

Python's yaml.laod can lead to code execution via crafted yaml files such as:

```
code_exec: !!python/object/apply:subprocess.check_output ['ls']
```

* Fixed XSS via bleach

It was possible to get an XSS via the markdown library via simply setting a description containing arbitary HTML tags.
It was also possible to create links that went to the `javascript:` link handler (eg `[example](javascript:alert(0)`)
Using bleach to sanitize it solves both of these.

* Added XFO header by default to prevent clickjacking attacks

Note that with this application clickjacking can be relatively severe via the SQLLab functionality
which allows executing arbitary SQL.

* Added justification for dangerouslySetInnerHTML

* Fixed linting errors

* Fixed linting errors
This commit is contained in:
David Dworken 2018-02-09 14:33:29 -08:00 committed by Maxime Beauchemin
parent 1769804ffd
commit 4ff17ffc8d
5 changed files with 22 additions and 5 deletions

View File

@ -80,6 +80,7 @@ setup(
'thrift>=0.9.3',
'thrift-sasl>=0.2.1',
'unidecode>=0.04.21',
'bleach==2.1.2',
],
extras_require={
'cors': ['Flask-Cors>=2.0.0'],

View File

@ -108,6 +108,12 @@ class GridCell extends React.PureComponent {
annotationQuery={annotationQuery}
/>
</div>
{
/* This usage of dangerouslySetInnerHTML is safe since it is being used to render
markdown that is sanitized with bleach. See:
https://github.com/apache/incubator-superset/pull/4390
and
https://github.com/apache/incubator-superset/commit/b6fcc22d5a2cb7a5e92599ed5795a0169385a825 */}
<div
className="slice_description bs-callout bs-callout-default"
style={isExpanded ? {} : { display: 'none' }}

View File

@ -221,7 +221,7 @@ def import_datasources(path, sync, recursive=False):
with f.open() as data_stream:
dict_import_export_util.import_from_dict(
db.session,
yaml.load(data_stream),
yaml.safe_load(data_stream),
sync=sync_array)
except Exception as e:
logging.error('Error when importing datasources from file %s', f)

View File

@ -277,10 +277,12 @@ SQL_CELERY_DB_FILE_PATH = os.path.join(DATA_DIR, 'celerydb.sqlite')
SQL_CELERY_RESULTS_DB_FILE_PATH = os.path.join(DATA_DIR, 'celery_results.sqlite')
# static http headers to be served by your Superset server.
# The following example prevents iFrame from other domains
# and "clickjacking" as a result
# HTTP_HEADERS = {'X-Frame-Options': 'SAMEORIGIN'}
HTTP_HEADERS = {}
# This header prevents iFrames from other domains and
# "clickjacking" as a result
HTTP_HEADERS = {'X-Frame-Options': 'SAMEORIGIN'}
# If you need to allow iframes from other domains (and are
# aware of the risks), you can disable this header:
# HTTP_HEADERS = {}
# The db id here results in selecting this one as a default in SQL Lab
DEFAULT_DB_ID = None

View File

@ -21,6 +21,7 @@ import sys
import uuid
import zlib
import bleach
import celery
from dateutil.parser import parse
from flask import flash, Markup, redirect, render_template, request, url_for
@ -433,11 +434,18 @@ def error_msg_from_exception(e):
def markdown(s, markup_wrap=False):
safe_markdown_tags = ['h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'b', 'i',
'strong', 'em', 'tt', 'p', 'br', 'span',
'div', 'blockquote', 'code', 'hr', 'ul', 'ol',
'li', 'dd', 'dt', 'img', 'a']
safe_markdown_attrs = {'img': ['src', 'alt', 'title'],
'a': ['href', 'alt', 'title']}
s = md.markdown(s or '', [
'markdown.extensions.tables',
'markdown.extensions.fenced_code',
'markdown.extensions.codehilite',
])
s = bleach.clean(s, safe_markdown_tags, safe_markdown_attrs)
if markup_wrap:
s = Markup(s)
return s