Safe Mode

[clphillips]

Parsedown offers no safe option for rendering, so, for those of us who allow no HTML content within markdown text, we must sanitize prior to feeding to Parsedown. As a result, Parsedown double encodes HTML entities (see #50).

Instead, Parsedown should offer a "safe" option.

function text($text, $safe_mode = false) {
    $this->safe_mode = $safe_mode;
    if ($safe_mode) {
        $text = htmlentities($text, ENT_QUOTES, 'UTF-8');
    }
    ...
}

Usage:

$parsedown = new ParseDown();
$text = "<script>alert('unsafe text');</script>";
echo $parsedown->text($text, true);

Output:

<script>alert(&quote;unsafe text&quote;);&lt/script>

[clphillips]

Also,

protected function identifyLink($excerpt)
{
    ...
    $url = $Link['url'];//str_replace(array('&', '<'), array('&amp;', '&lt;'), $Link['url']);
    ...

Should take into consideration safe_mode, and ignore this replacement.

[hkdobrev]

@clphillips Do you actually want Parsedown to have HTML-free mode?

[clphillips]

@hkdobrev Yes. HTML-free in terms of not allowing HTML content from untrusted users. In many cases the only HTML I want is what is produced by markdown flavored plain-text.

I'm actually hard pressed to find a single situation where I would actually want HTML content intermingled in my markdown text.

[apfelbox]

This is an absolute requirement to use this library as renderer for user-entered content.

[cebe]

you should use something like HTML purifier to extract all elements you do not want from the produced markdown.

[clphillips]

@cebe Uh, no we shouldn't. HTML purifier is extremely slow, and is a ridiculous solution for something that can be solved with 5 lines of code in this library.

[apfelbox]

@cebe there is absolutely no need to parse a huge DOM tree with a very complex library, when a simple text search & replace is sufficient. This is just waisting resources (and PHP is not exactly the fastest language available, so...).

Maybe we should also include securing link destinations:

[hi there](javascript:alert('hi there'))

[erusev]

I agree that it might make sense for Parsedown to do some sanitisation. It doesn't have to, but since it already knows so much about the texts it parses, it might not be too hard.

[clphillips]

Just for reference, other popular markdown libraries (for other languages) also support a "safe mode". See Python-Markdown and MarkdownDeep, for example.

Will try to update this issue with a pull request with the changes I've made when I get a chance.

[erusev]

@clphillips Thanks. I'll check them out.

[cebe]

I agree that the markdown parser can do it more easily.

[stewartlord]

Just adding fuel to the fire. Hoping to use Parsedown, but this is a blocker for me. I would also like to avoid using HTML Purifier for performance and licensing reasons. Happy to lend a hand.

[xPaw]

👍 Would also like to see safe mode.

[stewartlord]

Anyone started on this or are we looking for volunteers?

On May 25, 2014, at 10:15 AM, Pavel notifications@github.com wrote:

Would also like to see safe mode.

—
Reply to this email directly or view it on GitHub.

[erusev]

I'm working on it.

[stewartlord]

Awesome! 👍

On May 26, 2014, at 10:31 AM, Emanuil Rusev notifications@github.com wrote:

I'm working on it.

—
Reply to this email directly or view it on GitHub.

[xPaw]

I'm still waiting for this to be implemented before I can deploy parsedown on my website. Is there any progress?

[erusev]

Could we come up with a simple specification?

[xPaw]

Either have a whitelist of allowed elements (like ) and escape other elements, or just escape all html tags.

[erusev]

@xPaw This alone would not make the output safe. It's more complicated than that. What would we do with things like [link](javascript:alert('xss'))?

There's a related discussion at http://security.stackexchange.com/questions/14664/how-do-i-use-markdown-securely.

[xPaw]

Indeed urls have to be escaped as well. (shouldn't this already be done regardless of safe mode?)

[hkdobrev]

I suggest to have several setters for disabling certain functionality. One of them would disable usage of HTML in the Markdown. Another could be about XSS.

Then we could have a setter for safe-mode which will set several of those setters.

I am suggesting this approach, because some would want HTML-free mode without messing with parsing of URLs or images.

[markseuffert]

Good to hear a safe mode is in the works.

I am currently using Michel Fortin's Markdown parser and have done the following to prevent XSS: Disable HTML tags and entities, which escapes meta characters < and &. Filter Markdown generated URLs via callback url_filter_func(), which filters out unsafe things that are not on a whitelist. Then we tried to inject code with obfuscation and random data. A list of test cases can be found here: https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet

Edit: Updated link

[ckzhou]

[xss](http://"onmouseover="alert(1))

This vector can pop a alert box in firefox!

[xorock]

For several years I used Textile https://github.com/textile/php-textile and there is textileRestricted method which is suitable for untrusted input. As I understand there is no such thing in this library?

[erusev]

Would someone like to create a simple set of test data that includes .md and .html files with input and expected safe output?

[clphillips]

@erusev You want that as a PR, or what? Examples here: XSS Filter Evasion Cheat Sheet.

[erusev]

@clphillips Sure, but let's start simple. We could start with 5 to 10 pairs to see if this test driven approach would work.

[erusev]

Perhaps, we could start with something like: https://github.com/markdown-it/markdown-it/blob/019bbda5f5ee8b7d00f2633340aef3b0d000e3f1/test/fixtures/markdown-it/xss.txt

[dariuszp]

Hi, any ETA on this? I need markdown in my latest project. I was thinking of using parsedown but it will be for user generated content.
Because of this issue I had to abandon this library.

[BboyKeen]

I'm interested too. Any news for the safe mode ?

[erusev]

I've been thinking about this and I do plan to start implementing it, hopefully, in January.

[hootlex]

any update on this?

[erusev]

Not yet, but perhaps soon.

[hootlex]

Great, looking forward.

[Penagwin]

Is there any word on this right now?

[m1guelpf]

Updates?

[erusev]

Not yet, it's taking me some time :(

[sdkcarlos]

Well, for me worked perfectly by using the setMarkupEscaped option to true:

$markdown = "The markdown of the following image ... ";

$Parsedown = new Parsedown();

$Parsedown->setMarkupEscaped(true);

echo $Parsedown->text($markdown);

For example, the following markdown:

Would display in the browser the second alert if the setMarkupEscaped option isn't set to true. But if it does, it will generate the following HTML without being vulnerable to an XSS attack:

Maybe it was so simple and is not what everybody needs ... but it worked for me and maybe for someone else is useful too.

[aidantwoods]

Hi @sdkcarlos,

Apologies for being blunt here, I'd just like to make sure that no one gets the wrong impression.

it will generate the following HTML without being vulnerable to an XSS attack:

This is not the case. Parsedown is not safe with only setMarkupEscaped enabled.

Parsedown is still vulnerable to XSS attacks other than those introduced by inputting plain HTML tags, because Parsedown does not at present correctly encode data in attributes, nor does it sanitise link destinations (thus allowing use of the javascript: scheme).

I'd highly encourage you to take a look at #495 if you would like a version of Parsedown that is designed to be safe from XSS.

[sdkcarlos]

Thanks for the suggestion @aidantwoods , i've just updated my composer.json to require your version instead:

{
    "require": {
        "erusev/parsedown": "dev-master#bbb7687f31d6904f3a0e11e97bc61852a62cfe90",
    }
}

now i'm able to use setSafeMode and setMarkupEscaped 👍

[aidantwoods]

setSafeMode will flip all the switches that setMarkupEscaped would (so no need to set the latter – only setSafeMode is needed). Hopefully we can get that patch merged soon, and out as a proper release (updates are always nice – unfortunately can't really do that with fixed version).

Maybe use the following so that your composer will prefer the next stable Parsedown (once it is available)

"require": {
    "erusev/parsedown": "^1.7.0 || dev-master#bbb7687f31d6904f3a0e11e97bc61852a62cfe90",
}

(Assuming that the minor version would deserve a bump when this new feature is merged, @erusev
could confirm though).

[aidantwoods]

Hi @sdkcarlos,

I've implemented the anti-xss patch as a Parsedown extension, so you can now instead require aidantwoods/secureparsedown in your composer.json – which has the benefit of keeping both Parsedown and the anti-xss extension up to date.

Hopefully we'll see it in Parsedown core at some point though :)