RickStrahl / Westwind.AspNetCore

ASP.NET Core Helpers and Utilities
MIT License
129 stars 24 forks source link

Cross site scripting vulnerability #6

Closed Frankenleg closed 6 years ago

Frankenleg commented 6 years ago

It appears the markdown component is exposing cross site scripting vulnerabilities.

When the following text is placed inside the tag, the text is being rendered as script. Recycleablequp97<ScRiPt>alert(1)</ScRiPt>qgxsv

When the markdown tag is not used, it is rendered as text by .net: Recycleablequp97&lt;ScRiPt&gt;alert(1)&lt;/ScRiPt&gt;qgxsv

I have read the closed issue on this, but I feel like the markdown component should not be undoing the default behavior for rendering script tags.

I'm using .net core 2.1.2

Frankenleg commented 6 years ago

I was able to turn off HTML as recommended in the other issue (https://github.com/RickStrahl/Westwind.AspNetCore/issues/4).

I still think this should be considered a bug, because if I want to enable HTML in my markdown, I would then be vulnerable to XSS again. Markdown should not allow Javascript at all, even in HTML content.

RickStrahl commented 6 years ago

This is not a bug - generated Markdown should be treated like you would raw HTML. If you capture user input some sanitation has to happen.

It entirely depends on what you're doing with the markdown. If you're using standalone markdown pages, or even the tag helper with static content it's very likely you'd want scripts to render if they are part of the page. For example it's entirely possible to let the page handler embed script blocks to automate part of the page rendered by the Markdown.

Anyway, I've added some options to trigger the script stripping, and have replaced the script stripping code with a couple of more appropriate RegEx expressions.

The updates are here: 599a97cc541c055de27da0ab6c31b8a5e0fed6cb

RickStrahl commented 6 years ago

Added a number of more comprehensive mitigations for cross site scripting.