XSS vulnerabilities in comments marked as safe

[redyaffle]

Since all comments are marked as safe in markdown-rendered.html, someone could write Javascript as a comment and it gets interpreted as raw JS, opening up an XSS vulnerability. It seems like this is done so comments can be written in markdown, possibly for LaTeX as well (though removing |safe still seems to render LaTeX correctly). Do you consider the XSS vulnerability a problem? Would you accept a PR to fix it? If so, I was thinking that script tags could be stripped at time of submission, prior to posting to the database. You could also potentially strip all HTML period. It's not clear whether raw HTML should be necessary for any of the supported comment types.

Thank you!

Auto-reviewers: @NiharikaRay @matthewwardrop @earthmancash ancash @danfrankj

[matthewwardrop]

Hi @redyaffle,

I suspect that this is just one in a myriad of security vulnerabilities that exist throughout the knowledge repo codebase. In the past, this was not considered to be a particularly serious problem, since we internally used the knowledge repo in a trusted computing environment, but as it grows and matures, this sort of thing will be a big deal. I haven't yet personally reviewed much of the code pertaining to comments, so I cannot say exactly why we mark comment content as safe, but my guess is that as you say we render the content server-side before displaying it. It seems, therefore, reasonable to think about adding some functionality to strip unwanted tags/etc from the comment pre-rendering, and if you want to submit a patch that does this, it will be more than welcome!

Many things for getting involved!