search

showdownjs / ng-showdown

Angular integration for Showdown
BSD 3-Clause "New" or "Revised" License
105 stars 33 forks source link

Sanitize option not in docs #31

Open klummy opened 7 years ago

klummy commented 7 years ago

Hello,

I am not sure if this is a ng-showdown issue or Showdown issue but how to enable sanitization is missing from the docs. I had to look at multiple issues and the code itself to realise this.

Here is the option: $showdownProvider.setOption('sanitize', true)

rjmackay commented 6 years ago

I'd argue sanitize should default to enabled, as best practice to to have secure defaults.

tivie commented 6 years ago

Enabling "Sanitize" is kind of a "false sense of security" measure since, by itself, it does not protect against XSS attacks.

In order for it to be really secure you should follow extra steps. So enabling it by default could give the user a false sense of security. Also, it can break some legit use cases (where the user doesn't want any input to be sanitized).

Although I'm not against turning it on by default, there are legit concerns that the user might think showdown is XSS safe when it is not.

See https://github.com/showdownjs/showdown/wiki/Markdown's-XSS-Vulnerability-(and-how-to-mitigate-it) for more info.

mtrias commented 6 years ago

by itself, it does not protect against XSS attacks?

Could you please expand on why is this te case ?

Reading the link you pasted, it seems to me that applying ngSanitize to the output of showdown should be enough to prevent XSS, but it looks like I may be missing something.