Skip to content

Commit

Permalink
GHSL-2021-1017, GHSL-2021-1020, GHSL-2021-1021
Browse files Browse the repository at this point in the history
  • Loading branch information
martinRenou authored and SylvainCorlay committed Aug 9, 2022
1 parent 62f5c87 commit c90b746
Show file tree
Hide file tree
Showing 7 changed files with 155 additions and 12 deletions.
10 changes: 10 additions & 0 deletions nbconvert/exporters/html.py
Expand Up @@ -5,6 +5,7 @@

import base64
import json
from lxml.html.clean import clean_html
import mimetypes
import os
from pathlib import Path
Expand Down Expand Up @@ -149,6 +150,14 @@ def _template_name_default(self):
help="Template specific theme(e.g. the name of a JupyterLab CSS theme distributed as prebuilt extension for the lab template)",
).tag(config=True)

sanitize_html = Bool(
False,
help=(
"Whether the HTML in Markdown cells and cell outputs should be sanitized."
"This should be set to True by nbviewer or similar tools."
),
).tag(config=True)

embed_images = Bool(
False, help="Whether or not to embed images as base64 in markdown cells."
).tag(config=True)
Expand Down Expand Up @@ -287,4 +296,5 @@ def resources_include_url(name):
resources["jupyter_widgets_base_url"] = self.jupyter_widgets_base_url
resources["widget_renderer_url"] = self.widget_renderer_url
resources["html_manager_semver_range"] = self.html_manager_semver_range
resources["should_sanitize_html"] = self.sanitize_html
return resources
5 changes: 2 additions & 3 deletions nbconvert/exporters/templateexporter.py
Expand Up @@ -68,10 +68,9 @@
"get_metadata": filters.get_metadata,
"convert_pandoc": filters.convert_pandoc,
"json_dumps": json.dumps,
# browsers will parse </script>, closing a script tag early
# Since JSON allows escaping forward slash, this will still be parsed by JSON
"escape_html_script": lambda x: x.replace("</script>", "<\\/script>"),
# For removing any HTML
"escape_html": html.escape,
# For sanitizing HTML for any XSS
"clean_html": clean_html,
"strip_trailing_newline": filters.strip_trailing_newline,
"text_base64": filters.text_base64,
Expand Down
63 changes: 63 additions & 0 deletions nbconvert/exporters/tests/files/notebook_inject.ipynb
Expand Up @@ -191,6 +191,69 @@
}
],
"source": [""]
},
{
"cell_type": "code",
"execution_count": 5,
"id": "2616e107",
"metadata": {},
"outputs": [
{
"data": {
"text/html": [
"<script>alert('text/html output')</script>"
]
},
"execution_count": 5,
"metadata": {},
"output_type": "execute_result"
}
],
"source": [
"import os; os.system('touch /tmp/pwned')"
]
},
{
"cell_type": "code",
"execution_count": 5,
"id": "3616e107",
"metadata": {},
"outputs": [
{
"data": {
"text/markdown": [
"<script>alert('text/markdown output')</script>"
]
},
"execution_count": 5,
"metadata": {},
"output_type": "execute_result"
}
],
"source": [
"import os; os.system('touch /tmp/pwned')"
]
},
{
"cell_type": "code",
"execution_count": 5,
"id": "4616e107",
"metadata": {},
"outputs": [
{
"data": {
"application/javascript": [
"alert('application/javascript output')"
]
},
"execution_count": 5,
"metadata": {},
"output_type": "execute_result"
}
],
"source": [
"import os; os.system('touch /tmp/pwned')"
]
}
],
"metadata": {
Expand Down
29 changes: 27 additions & 2 deletions nbconvert/exporters/tests/test_html.py
Expand Up @@ -137,7 +137,9 @@ def test_basic_name(self):

def test_javascript_injection(self):
for template in ["lab", "classic", "reveal"]:
(output, resources) = HTMLExporter(template_name=template).from_filename(self._get_notebook('notebook_inject.ipynb'))
(output, resources) = HTMLExporter(
template_name=template
).from_filename(self._get_notebook('notebook_inject.ipynb'))

# Check injection in the metadata.title of the Notebook
assert "<script>alert('title')</script>" not in output
Expand All @@ -150,7 +152,6 @@ def test_javascript_injection(self):

# Check injection in the cell.source of the Notebook
assert "<script>alert('raw cell')</script>" not in output
assert "<script>alert('markdown cell')</script>" not in output

# Check injection in svg output
assert "<script>alert('image/svg+xml output')</script>" not in output
Expand All @@ -170,3 +171,27 @@ def test_javascript_injection(self):

# Check injection in widget view
assert "<script>alert('output.data.application/vnd.jupyter.widget-view+json injection')" not in output

# By design, text/html, text/markdown, application/javascript and markdown cells should allow
# for JavaScript code execution
for template in ["lab", "classic", "reveal"]:
(output, resources) = HTMLExporter(
template_name=template
).from_filename(self._get_notebook('notebook_inject.ipynb'))

assert "<script>alert('markdown cell')</script>" in output
assert "<script>alert('text/markdown output')</script>" in output
assert "<script>alert('text/html output')</script>" in output
assert "alert('application/javascript output')" in output

# But it's an opt-out
for template in ["lab", "classic", "reveal"]:
(output, resources) = HTMLExporter(
template_name=template,
sanitize_html=True
).from_filename(self._get_notebook('notebook_inject.ipynb'))

assert "<script>alert('markdown cell')</script>" not in output
assert "<script>alert('text/markdown output')</script>" not in output
assert "<script>alert('text/html output')</script>" not in output
assert "alert('application/javascript output')" not in output
9 changes: 9 additions & 0 deletions nbconvert/nbconvertapp.py
Expand Up @@ -65,6 +65,7 @@ def validate(self, obj, value):
"template": "TemplateExporter.template_name",
"template-file": "TemplateExporter.template_file",
"theme": "HTMLExporter.theme",
"sanitize_html": "HTMLExporter.sanitize_html",
"writer": "NbConvertApp.writer_class",
"post": "NbConvertApp.postprocessor_class",
"output": "NbConvertApp.output_base",
Expand Down Expand Up @@ -178,6 +179,14 @@ def validate(self, obj, value):
},
"""Embed the images as base64 dataurls in the output. This flag is only useful for the HTML/WebPDF/Slides exports.""",
),
"sanitize-html": (
{
"HTMLExporter": {
"sanitize_html": True,
}
},
"""Whether the HTML in Markdown cells and cell outputs should be sanitized..""",
),
}
)

Expand Down
31 changes: 26 additions & 5 deletions share/templates/classic/base.html.j2
Expand Up @@ -81,7 +81,12 @@
{%- endif -%}
<div class="inner_cell">
<div class="text_cell_render border-box-sizing rendered_html">
{{ cell.source | markdown2html | strip_files_prefix | clean_html }}
{%- if resources.should_sanitize_html %}
{%- set html_value=cell.source | markdown2html | strip_files_prefix | clean_html -%}
{%- else %}
{%- set html_value=cell.source | markdown2html | strip_files_prefix -%}
{%- endif %}
{{ html_value }}
</div>
</div>
</div>
Expand Down Expand Up @@ -133,23 +138,33 @@ unknown type {{ cell.type }}

{% block data_html scoped -%}
<div class="output_html rendered_html output_subarea {{ extra_class }}">
{%- if resources.should_sanitize_html %}
{%- set html_value=output.data['text/html'] | clean_html -%}
{%- else %}
{%- set html_value=output.data['text/html'] -%}
{%- endif %}
{%- if output.get('metadata', {}).get('text/html', {}).get('isolated') -%}
<iframe
class="isolated-iframe"
style="height:520px; width:100%; margin:0; padding: 0"
frameborder="0"
scrolling="auto"
src="data:text/html;base64,{{output.data['text/html'] | text_base64}}">
src="data:text/html;base64,{{ html_value | text_base64 }}">
</iframe>
{%- else -%}
{{ output.data['text/html'] }}
{{ html_value }}
{%- endif -%}
</div>
{%- endblock data_html %}

{% block data_markdown scoped -%}
{%- if resources.should_sanitize_html %}
{%- set html_value=output.data['text/markdown'] | markdown2html | clean_html -%}
{%- else %}
{%- set html_value=output.data['text/markdown'] | markdown2html -%}
{%- endif %}
<div class="output_markdown rendered_html output_subarea {{ extra_class }}">
{{ output.data['text/markdown'] | markdown2html }}
{{ html_value }}
</div>
{%- endblock data_markdown %}

Expand Down Expand Up @@ -234,14 +249,17 @@ alt="{{ alttext | escape_html }}"
{%- block data_javascript scoped %}
{% set div_id = uuid4() %}
<div id="{{ div_id }}" class="output_subarea output_javascript {{ extra_class }}">
{%- if not resources.should_sanitize_html %}
<script type="text/javascript">
var element = $('#{{ div_id }}');
{{ output.data['application/javascript'] }}
</script>
{%- endif %}
</div>
{%- endblock -%}

{%- block data_widget_view scoped %}
{%- if not resources.should_sanitize_html %}
{% set div_id = uuid4() %}
{% set datatype_list = output.data | filter_data_type %}
{% set datatype = datatype_list[0]%}
Expand All @@ -253,14 +271,17 @@ var element = $('#{{ div_id }}');
{{ output.data[datatype] | json_dumps | escape_html }}
</script>
</div>
{%- endif %}
{%- endblock data_widget_view -%}

{%- block footer %}
{%- if not resources.should_sanitize_html %}
{% set mimetype = 'application/vnd.jupyter.widget-state+json'%}
{% if mimetype in nb.metadata.get("widgets",{})%}
<script type="{{ mimetype }}">
{{ nb.metadata.widgets[mimetype] | json_dumps | clean_html }}
{{ nb.metadata.widgets[mimetype] | json_dumps | escape_html }}
</script>
{% endif %}
{%- endif %}
{{ super() }}
{%- endblock footer-%}
20 changes: 18 additions & 2 deletions share/templates/lab/base.html.j2
Expand Up @@ -102,7 +102,12 @@
{{ self.empty_in_prompt() }}
{%- endif -%}
<div class="jp-RenderedHTMLCommon jp-RenderedMarkdown jp-MarkdownOutput {{ celltags(cell) }}" data-mime-type="text/markdown">
{{ cell.source | markdown2html | strip_files_prefix | clean_html }}
{%- if resources.should_sanitize_html %}
{%- set html_value=cell.source | markdown2html | strip_files_prefix | clean_html -%}
{%- else %}
{%- set html_value=cell.source | markdown2html | strip_files_prefix -%}
{%- endif %}
{{ html_value }}
</div>
</div>
</div>
Expand Down Expand Up @@ -165,13 +170,22 @@ unknown type {{ cell.type }}

{% block data_html scoped -%}
<div class="jp-RenderedHTMLCommon jp-RenderedHTML jp-OutputArea-output {{ extra_class }}" data-mime-type="text/html">
{%- if resources.should_sanitize_html %}
{{ output.data['text/html'] | clean_html }}
{%- else %}
{{ output.data['text/html'] }}
{%- endif %}
</div>
{%- endblock data_html %}

{% block data_markdown scoped -%}
{%- if resources.should_sanitize_html %}
{%- set html_value=output.data['text/markdown'] | markdown2html | clean_html -%}
{%- else %}
{%- set html_value=output.data['text/markdown'] | markdown2html -%}
{%- endif %}
<div class="jp-RenderedHTMLCommon jp-RenderedMarkdown jp-OutputArea-output {{ extra_class }}" data-mime-type="text/markdown">
{{ output.data['text/markdown'] | markdown2html }}
{{ html_value }}
</div>
{%- endblock data_markdown %}

Expand Down Expand Up @@ -270,10 +284,12 @@ jp-needs-dark-background
{%- block data_javascript scoped %}
{% set div_id = uuid4() %}
<div id="{{ div_id }}" class="jp-RenderedJavaScript jp-OutputArea-output {{ extra_class }}" data-mime-type="application/javascript">
{%- if not resources.should_sanitize_html %}
<script type="text/javascript">
var element = document.getElementById('{{ div_id }}');
{{ output.data['application/javascript'] }}
</script>
{%- endif %}
</div>
{%- endblock -%}

Expand Down

0 comments on commit c90b746

Please sign in to comment.