Security issue - Vulnerable to XSS attack #163

LzOggar · 2019-10-03T22:30:14Z

Description

A security problem exists when markdownx.js dynamically interprets the content of the form without first checking the content. The application is vulnerable to XSS attack.

Versions

django 1.11.13 and 2.2.5,
python 2.7.16 and 3.7.4.
django-markdownx 2.0.28.

Preview

xenatisch · 2019-10-04T08:04:09Z

Hi @LzOggar

Thanks for reporting this. This, however, is not something new. It is more of a design dilemma, and has been since I started my contributions to this project. Here's a summary:

It has been argued that this is not an issue for MarkdownX to address. After all, it is the Markdown transpiler that should filter this out. MarkdownX is but a medium.
If we attempt to filter all HTML tags, it would mean that developers won't have the flexibility to incorporate custom tags into their Markdown editors. Whilst tempting for obvious, I do not think that it is a decision for us, but one for the developer. After all, this is not meant to be the end product.

So what now?
I think the best course of action would be to:

Warn developers of the possibility of XSS -- e.g. using your very informative GIF.
Come up with a settings variable to block all HTML tags and render them as text -- note that we cannot check every attribute of every tag to ensure there's no JS code. It take too much resources. Although that option is open the developers.

I am not particularly supportive of the latter option because it invades a jurisdiction that belongs to the transpiler. Indeed most, if not all decent transpilers offer the option to ignore raw HTML. If we decide to go down that route, it could get very messy, very soon. That's because if we want to support all transpilers, we're gonna have to parse the resulting HTML file, and that's a can of worms no one wants to open.

Anyhow, these are my thoughts. Happy to hear yours.

eon01 · 2019-10-09T00:12:03Z

I was trying to discover if a user can inject JS and the surprise was that I was able to pop up alerts and do everything JS can do. So I googled this and landed here.
I was also able to add things like:

<body onfocus="alert('')"></body>

or

<meta http-equiv=refresh content=1>

or anything that could block or freeze the page, the browser or the OS ..etc

I think security, in this case, should come before features.
I'm using Bleach now in the view to clean all suspect JS code but would love to hear if there are better solutions.

LzOggar · 2019-10-09T15:47:31Z

Hi,

firstable, I hope you are using whitelist with Bleach.clean function.

More over, you can sanitize all user input with Bleach and save result in db. The render will not execute any arbitrary code. But, i don't know if you patch js middleware with Bleach. So, it could be not working with dynamic render.

Please, let me know more about your patch. Thanks you.

Regards.

eon01 · 2019-10-12T02:26:38Z

For the rendering, I think it's possible if you create a template tag, something like:

from django import template
register = template.Library()
import bleach

@register.filter
def sanitize(value):
    return bleach.clean(value)

then override the widget template (markdownx/widget2.html):

<div class="markdownx">
    {{ markdownx_editor }}
    <div class="markdownx-preview"></div>
</div>

and use JS to apply the template tag to the preview DIV. I didn't test this yet, but it could help.

LzOggar · 2019-10-13T09:38:39Z

So, on your advice, I tried and I did not success then I found new way and I got it. Let's explain ;

Add new global var to "markdownx/settings" as you can see here :
Edit "markdownx/utils", modify "mardownify" function as well as you can see here :

and save it.

Enjoy ! Here is a valid way and I think the best way to patch this. But, i am not a really fan because, we have to change the default code of markdownx.

eon01 · 2019-10-13T13:53:41Z

You can do it without modifying the package code by overriding the markdownify() function in your settings.py:

Create a file called utils.py and add your new markdownify() and keep the original one.

MARKDOWNX_MARKDOWNIFY_FUNCTION = '<myapp>.utils.markdownify'

By the way, the new markdownify you used will strip HTML like:

<a href="#">Click</a>
<a href="javascript:alert('XSS')">Click</a>

But not JS inside markdown:

[test](javascript:alert\('test'\))

This is what I used:

from functools import partial


from django.conf import settings
from django.utils.safestring import mark_safe

import markdown
import bleach


def markdownify(text):

    # Bleach settings
    whitelist_tags = getattr(settings, 'MARKDOWNIFY_WHITELIST_TAGS', bleach.sanitizer.ALLOWED_TAGS)
    whitelist_attrs = getattr(settings, 'MARKDOWNIFY_WHITELIST_ATTRS', bleach.sanitizer.ALLOWED_ATTRIBUTES)
    whitelist_styles = getattr(settings, 'MARKDOWNIFY_WHITELIST_STYLES', bleach.sanitizer.ALLOWED_STYLES)
    whitelist_protocols = getattr(settings, 'MARKDOWNIFY_WHITELIST_PROTOCOLS', bleach.sanitizer.ALLOWED_PROTOCOLS)

    # Markdown settings
    strip = getattr(settings, 'MARKDOWNIFY_STRIP', True)
    extensions = getattr(settings, 'MARKDOWNIFY_MARKDOWN_EXTENSIONS', [])

    # Bleach Linkify
    linkify = None
    linkify_text = getattr(settings, 'MARKDOWNIFY_LINKIFY_TEXT', True)

    if linkify_text:
        linkify_parse_email = getattr(settings, 'MARKDOWNIFY_LINKIFY_PARSE_EMAIL', False)
        linkify_callbacks = getattr(settings, 'MARKDOWNIFY_LINKIFY_CALLBACKS', None)
        linkify_skip_tags = getattr(settings, 'MARKDOWNIFY_LINKIFY_SKIP_TAGS', None)
        linkifyfilter = bleach.linkifier.LinkifyFilter

        linkify = [partial(linkifyfilter,
                callbacks=linkify_callbacks,
                skip_tags=linkify_skip_tags,
                parse_email=linkify_parse_email
                )]

    # Convert markdown to html
    html = markdown.markdown(text, extensions=extensions)

    # Sanitize html if wanted
    if getattr(settings, 'MARKDOWNIFY_BLEACH', True):

        cleaner = bleach.Cleaner(tags=whitelist_tags,
                                 attributes=whitelist_attrs,
                                 styles=whitelist_styles,
                                 protocols=whitelist_protocols,
                                 strip=strip,
                                 filters=linkify,
                                 )

        html = cleaner.clean(html)

    return mark_safe(html)

It's the same function here.

In your settings you should add the following configurations and customize them according to your needs:

## markdownify
MARKDOWNIFY_WHITELIST_TAGS = [
  'a',
  'abbr',
  'acronym',
  'b',
  'blockquote',
  'em',
  'i',
  'li',
  'ol',
  'p',
  'strong',
  'ul',
  'pre',
  'code',
]
MARKDOWNIFY_WHITELIST_PROTOCOLS = [
    'http',
    'https',
]
MARKDOWNIFY_LINKIFY_PARSE_EMAIL = True
MARKDOWNIFY_LINKIFY_SKIP_TAGS = ['pre', 'code', ]
MARKDOWNIFY_WHITELIST_ATTRS = [
    'href',
    'src',
    'alt',
    'class',
]

LzOggar · 2019-10-18T06:10:41Z

The solution seems worked fine. Case closed.

jonathan-daniel · 2020-05-11T17:28:19Z

Very irresponsible to not mention this in the docs

LzOggar closed this as completed Oct 18, 2019

jonathan-daniel mentioned this issue May 11, 2020

[SOLVED] Render markdown to HTML in template #83

Closed

ochan1 mentioned this issue Jan 1, 2021

Markdown Security Patch compserv/hknweb#280

Merged

adi- pinned this issue Feb 28, 2022

adi- mentioned this issue Jun 12, 2023

Cross-Site Scripting (XSS) Vulnerability in Markdown Library #255

Closed

adi- unpinned this issue Jul 21, 2023

adi- mentioned this issue May 21, 2024

just now fix! severity found #265

Closed

adi- pinned this issue May 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Security issue - Vulnerable to XSS attack #163

Security issue - Vulnerable to XSS attack #163

LzOggar commented Oct 3, 2019

xenatisch commented Oct 4, 2019

eon01 commented Oct 9, 2019

LzOggar commented Oct 9, 2019

eon01 commented Oct 12, 2019

LzOggar commented Oct 13, 2019

eon01 commented Oct 13, 2019

LzOggar commented Oct 18, 2019

jonathan-daniel commented May 11, 2020

Security issue - Vulnerable to XSS attack #163

Security issue - Vulnerable to XSS attack #163

Comments

LzOggar commented Oct 3, 2019

Description

Versions

Preview

xenatisch commented Oct 4, 2019

eon01 commented Oct 9, 2019

LzOggar commented Oct 9, 2019

eon01 commented Oct 12, 2019

LzOggar commented Oct 13, 2019

eon01 commented Oct 13, 2019

LzOggar commented Oct 18, 2019

jonathan-daniel commented May 11, 2020