XSS in markdown filter

[magicOz]

NetBox version

v3.2.2

Python version

3.8

Steps to Reproduce

The markdown-filter uses Python-Markdown with the Fenced Code Blocks extension (https://python-markdown.github.io/extensions/fenced_code_blocks/) to render markdown formatted text. ( https://github.com/netbox-community/netbox/blob/8d682041a43b6176198f64bd80a46ea9ed99d2d8/netbox/utilities/templatetags/builtins/filters.py#L140,L167 )

It is possible to break out of the HTML-attributes added by the fenced code blocks extension - to form a new, arbitrary, HTML-tag. Since the rendering of markdown HTML occurs after the stripping of HTML-tags, this will avoid the sanitization made by django.utils.html.strip_tags.

The following payload will trigger a XSS wherever the markdown-filter is being used:

``` { ."><script/}
alert(/XSS/);
/*
```
``` { .html*/</script/}
```

Expected Behavior

If possible - sanitizate the end-result of the markdown processing, so that even if Python-Markdown fails to strip or incorrectly format HTML an attacker wouldn't be able to abuse it.

Observed Behavior

XSS

[kkthxbye-code]

Did you report this upstream? I can replicate it using the python-markdown CLI.

Edit: I see you reported it upstream while I was typing up this response. Let's see what their response is.

If possible - sanitizate the end-result of the markdown processing, so that even if Python-Markdown fails to strip or incorrectly format HTML an attacker wouldn't be able to abuse it.

I'm not sure if I follow here. Maybe you are missing the purpose of markdown parsing, which is to generate HTML. If you sanitize the end-result (you mention django.utils.html.strip_tags so I assume that's what you mean by sanitize), you'll end up with just text, negating the entire point of markdown parsing.

[magicOz]

Did you report this upstream? I can replicate it using the python-markdown CLI.

Yes, Python-Markdown/markdown#1247

I'm not sure if I follow here. Maybe you are missing the purpose of markdown parsing, which is to generate HTML. If you sanitize the end-result (you mention django.utils.html.strip_tags so I assume that's what you mean by sanitize), you'll end up with just text, negating the entire point of markdown parsing.

Right, so what I meant by sanitize is not to strip the end-result of all HTML tags but to only allow tags and attributes that the markdown processor is expected to generate.

[kkthxbye-code]

Right, so what I meant by sanitize is not to strip the end-result of all HTML tags but to only allow tags and attributes that the markdown processor is expected to generate.

That would be just as hard as doing it right in python-markdown in the first place. You could duct tape something like bleach on, but then you just have two libraries which are prone to bugs letting stuff slip through.

If you have a suggestion of a more robust implementation of markdown parsing, please feel free to share it. For now I think we'll have to wait to see what upstream says to your report.

[waylan]

@kkthxbye-code, @magicOz is correct. You need to sanitize the HTML output of any Markdown parser if you are processing input from untrusted sources. For a clear explanation of why, I would suggest reading Markdown and XSS by Michel Fortin, the developer of PHP Markdown. Therefore, Python-Markdown does not promise (and neither does any Markdown implementation that I am aware of) that our output will be XSS safe. That is the responsibility of the user. That said, I'm not opposed to addressing this specific edge case as discussed in the upstream issue.

[kkthxbye-code]

@waylan - Fair enough. As I said we will certainly accept contributions regarding a more rubust implementation to markdown parsing in general. It has also been discussed in the past:

#7788

https://github.com/netbox-community/netbox/issues?q=is%3Aissue+xss+

This specific issue will probably wait for your decision upstream.

[kkthxbye-code]

This has been fixed upstream in https://github.com/Python-Markdown/markdown/releases/tag/3.3.7 (thanks @waylan). This can be closed when you update dependencies for 3.3 @jeremystretch

[jeremystretch]

Will bump Markdown to v3.3.7 as part of the usual release process for NetBox v3.2.3.