Erudika / scoold

A Stack Overflow clone for teams (self-hosted or hosted)
https://scoold.com
Apache License 2.0
863 stars 239 forks source link

Possible security risk on mentions #322

Closed fpellet closed 2 years ago

fpellet commented 2 years ago

Hello,

I have seen this change baaacfb69bcc986e5c90dd4523c84b84cf66f228

It is possible to inject content. For example: warning: @<overridelink" onclick="console.log(5);return false;"|dangerous link> It is partially possible to override the link. Normally JS is not executed with CSP, but I think it is still risky.

albogdano commented 2 years ago

Thank you for finding that Florent! I agree it is dangerous if the CSP header is turned off (it's on by default). Perhaps we can sanitize the text inside the link and remove symbols like " ( ) ; '? I'm sure there are other holes in the clientside code which are only covered by the CSP, so it's kind of critical in my opinion.

fpellet commented 2 years ago

Maybe don't accept quote and encodeURI in group 2, and escape html in group 3 (https://stackoverflow.com/questions/3043775/how-to-escape-html)

I agree that CSP should not be disabled, but I see it more as a safety net. I have more peace of mind :)