Commit Graph

9 Commits

Author SHA1 Message Date
Javi Martín
00a6f5b601 Remove <%== usage displaying settings
Using `<%==` is the same as using `raw`. I'm not sure if we meant
`sanitize` in this case, or it's just a typo. I'm assuming the latter
since we don't use anything similar in any other places.
2019-10-08 19:10:14 +02:00
Javi Martín
9eee79f218 Sanitize markdown output
We were using the markdown renderer with the `filter_html` option set to
false, so we weren't removing hypothetical `<script>` tags.
2019-10-08 18:46:21 +02:00
Javi Martín
928312e218 Use sanitize in translations with links
Sometimes we're interpolating a link inside a translation, and marking
the whole translations as HTML safe.

However, some translations added by admins to the database or through
crowdin are not entirely under our control.

Although AFAIK crowdin checks for potential cross-site scripting
attacks, it's a good practice to sanitize parts of a string potentially
out of our control before marking the string as HTML safe.
2019-10-08 18:46:21 +02:00
Javi Martín
75a28fafcb Sanitize label texts automatically
This way we can remove all those `html_safe` calls and we avoid
potential XSS attacks in label texts.
2019-10-08 18:46:21 +02:00
Javi Martín
2ffbae890e Sanitize valuation explanations
If we don't sanitize them, valuators might attempt Cross-Site Scripting
attacks.
2019-10-08 18:46:20 +02:00
Javi Martín
8b73cfc019 Sanitize annotation context before displaying it
There's a case where we would face a Cross-Site Scripting attack. An
attacker could use the browser's developer tools to add (on their
browser) a `<code>` tag with a `<script>` tag inside in the text of the
draft version. After doing so, commenting on that text would result in
the attacker's JavaScript being executed.
2019-10-08 18:46:20 +02:00
Javi Martín
0f485308b7 Sanitize CKEditor content before displaying it
It's possible to create a newsletter or a proposed action with
<script> tags by filling in the body using a textarea instead of a
CKEditor. While we trust our administrators not to do so, it's better to
completely eliminate that possibility.
2019-10-08 18:46:20 +02:00
Javi Martín
db1ccb18c7 Use safe_join instead of html_safe
The name `html_safe` is very confusing, and many developers (including
me a few years ago) think what that method does is convert the HTML
contents to safe content. It's actually quite the opposite: it marks the
string as safe, so the HTML inside it isn't stripped out by Rails.

In some cases we were marking strings as safe because we wanted to add
some HTML. However, it meant the whole string was considered safe, and
not just the contents which were under our control.

In particular, some translations added by admins to the database or
through crowding were marked as safe, when it wasn't necessarily the
case.

Although AFAIK crowdin checks for potential cross-site scripting
attacks, it's a good practice to sanitize parts of a string potentially
out of our control before marking the string as HTML safe.
2019-10-08 18:46:20 +02:00
Javi Martín
20ca6beb30 Remove unneeded html_safe and raw calls
There's no HTML in these texts, or it has already been escaped by Rails
`link_to` helper method.
2019-10-08 13:20:22 +02:00