starred/netbox
1
0
Fork 0
mirror of https://github.com/netbox-community/netbox.git synced 2026-01-12 05:20:31 +01:00
Code Issues 273 Packages Projects Releases 10 Wiki Activity

XSS in markdown filter #6442

New Issue
Closed
opened 2025-12-29 19:40:47 +01:00 by adam · 7 comments
adam commented 2025-12-29 19:40:47 +01:00
Owner
Copy Link

Originally created by @magicOz on GitHub (May 4, 2022).

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. ( 8d682041a4/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

Originally created by @magicOz on GitHub (May 4, 2022). ### 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
adam added the type: bug label 2025-12-29 19:40:47 +01:00
adam closed this issue 2025-12-29 19:40:47 +01:00
adam commented 2025-12-29 19:40:47 +01:00
Author
Owner
Copy Link

@kkthxbye-code commented on GitHub (May 4, 2022):

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.

@kkthxbye-code commented on GitHub (May 4, 2022): 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.
adam commented 2025-12-29 19:40:47 +01:00
Author
Owner
Copy Link

@magicOz commented on GitHub (May 4, 2022):

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

Yes, https://github.com/Python-Markdown/markdown/issues/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.

@magicOz commented on GitHub (May 4, 2022): > Did you report this upstream? I can replicate it using the python-markdown CLI. Yes, https://github.com/Python-Markdown/markdown/issues/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.
adam commented 2025-12-29 19:40:48 +01:00
Author
Owner
Copy Link

@kkthxbye-code commented on GitHub (May 4, 2022):

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.

@kkthxbye-code commented on GitHub (May 4, 2022): > 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](https://pypi.org/project/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.
adam commented 2025-12-29 19:40:48 +01:00
Author
Owner
Copy Link

@waylan commented on GitHub (May 4, 2022):

@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.

@waylan commented on GitHub (May 4, 2022): @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][1] 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. [1]: https://michelf.ca/blog/2010/markdown-and-xss/
adam commented 2025-12-29 19:40:48 +01:00
Author
Owner
Copy Link

@kkthxbye-code commented on GitHub (May 4, 2022):

@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:

https://github.com/netbox-community/netbox/issues/7788

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

This specific issue will probably wait for your decision upstream.

@kkthxbye-code commented on GitHub (May 4, 2022): @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: https://github.com/netbox-community/netbox/issues/7788 https://github.com/netbox-community/netbox/issues?q=is%3Aissue+xss+ This specific issue will probably wait for your decision upstream.
adam commented 2025-12-29 19:40:48 +01:00
Author
Owner
Copy Link

@kkthxbye-code commented on GitHub (May 6, 2022):

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

@kkthxbye-code commented on GitHub (May 6, 2022): 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
adam commented 2025-12-29 19:40:49 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (May 11, 2022):

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

@jeremystretch commented on GitHub (May 11, 2022): Will bump Markdown to v3.3.7 as part of the usual release process for NetBox v3.2.3.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
beta
breaking change
complexity: high
complexity: low
complexity: medium
needs milestone
netbox
pending closure
plugin candidate
pull-request
Mirrored from GitHub Pull Request
 severity: high
severity: low
severity: medium
status: accepted
status: backlog
status: blocked
status: duplicate
status: needs owner
status: needs triage
status: revisions needed
status: under review
topic: GraphQL
topic: Internationalization
topic: OpenAPI
topic: UI/UX
topic: cabling
topic: event rules
topic: htmx navigation
topic: industrialization
topic: migrations
topic: plugins
topic: scripts
topic: templating
topic: testing
type: bug
type: deprecation
type: documentation
type: feature
type: housekeeping
type: translation
No Label type: bug
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/netbox#6442
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?