django-markdown-editor icon indicating copy to clipboard operation
django-markdown-editor copied to clipboard

XSS via image

Open wfinn opened this issue 3 years ago • 1 comments

Hi, I found another XSS which is different from the javascript: one.

![\" onerror=alert(1) ](x) and it results in this html: <p><img alt="" onerror=alert(1) " src="x" /></p>

I think you should use a well known HTML sanitizer after generating the HTML from markdown to fix this. That would also fix #167

But preventing breaking out of the html with \" should be done aswell.

wfinn avatar Jan 11 '22 13:01 wfinn

very good @wfinn, I guess you are pentester... lol

agusmakmun avatar Jan 12 '22 01:01 agusmakmun

I think these can all be tackled by a strict enough content security policy, not allowing inline scripts. A package I use to configure this in my apps is django-csp.

However, I do agree a sanitizer would be kind of nice. We could for example add bleach to the mix.

eelkevdbos avatar Nov 17 '22 20:11 eelkevdbos

Now finally we solve this issue by using bleach, thank you @eelkevdbos for your recommendation. Please upgrade your package by using this command:

pip install martor --upgrade

meanwhile, I already provide the unitest as well which related with xss issues:

https://github.com/agusmakmun/django-markdown-editor/blob/master/martor/tests/tests.py#L80

def test_markdownify_xss_handled(self):
    xss_payload_1 = "[aaaa](javascript:alert(1))"
    response_1 = markdownify(xss_payload_1)
    self.assertEqual(response_1, '<p><a href=":">aaaa</a></p>')

    xss_payload_2 = '![" onerror=alert(1) ](x)'
    response_2 = markdownify(xss_payload_2)
    self.assertEqual(
        response_2, '<p><img alt="&quot; onerror=alert(1) " src="x"></p>'
    )

    xss_payload_3 = '[xss](" onmouseover=alert(document.domain) l)'
    response_3 = markdownify(xss_payload_3)
    self.assertEqual(
        response_3,
        '<p><a href="&quot; onmouseover=alert(document.domain)">xss</a>)</p>',  # noqa: E501
    )

agusmakmun avatar Nov 18 '22 14:11 agusmakmun