search

CruGlobal / conf-registration-web

Event Registration Tool
https://www.eventregistrationtool.com
MIT License
2 stars 1 forks source link

EVENT-781-Improvement #781

Closed TheNoodleMoose closed 2 years ago

TheNoodleMoose commented 2 years ago

After talking with some of the people from the ERT team, I recommended it would be better for safety reasons to not use toTrustedHTML. They do lose the ability to use CSS, but the security benefits we gain outway that utility I believe.

Security reasons behind this are the fact that $sce.trustAsHtml overrides the ngSanitize that should normally happen. This means whatever is inputted, is not checked for anything that could be 'unsafe'. The sanitize logic essentially strips out everything that is not basic HTML code, including css, scripts, etc.

With overriding the sanitize logic, I can do the following:

Screen Shot 2022-01-31 at 10 24 37 AM

and since it's not scrubbed out, it gets executed

Screen Shot 2022-01-31 at 10 24 46 AM
wrandall22 commented 2 years ago

Can you list here the security benefits you're hoping to gain from this move? Also, is it worth it in the future to allow CSS in some other, more controlled way?

TheNoodleMoose commented 2 years ago

@wrandall22 Went ahead and updated the description. And after talking with the ERT team, they're not too worried about using CSS in the description. We do actually already allow for custom CSS stylesheets, so I believe they could utilize that if they ever had the desire to.