search

jdabtieu / CTFOJ

Lightweight CTF judge platform for capture-the-flag (CTF) clubs
GNU Affero General Public License v3.0
10 stars 5 forks source link

[FEATURE] (Admin) Input Sanitization #37

Closed jdabtieu closed 3 years ago

jdabtieu commented 3 years ago

Is your feature request related to a problem? Please describe.

Related to #31 Poorly formatted Markdown can destroy the entire page, as seen in Anthony's screenshot. Javascript should be BANNED, otherwise someone can take down the site by simple injecting window.location.href="http://attacker.url", which would redirect people to http://attacker.url when the script executes

Describe the solution you'd like

External library to sanitize all Markdown-converted text: https://github.com/cure53/DOMPurify Making our own is a terrible idea

Describe alternatives you've considered

No

Additional context

Cookies cannot be stolen as they have the HttpOnly attribute.

However, a phising attack on an admin would allow an attacker to inject harmful code that can cause a DOS, among other exploits.

slightlyskepticalpotat commented 3 years ago

I've also noticed other admin-facing fields aren't sanitised.

jdabtieu commented 3 years ago

I've also noticed other admin-facing fields aren't sanitised.

Apart from the markdown ones, which other fields aren't sanitized?

slightlyskepticalpotat commented 3 years ago

See #38 and #39, but also the editing/creating problems interface. Of course, if they aren't messing around on purpose they shouldn't run into the errors.

jdabtieu commented 3 years ago

Ah, ok. I classified them as markdown-enabled fields, since they are markdown-enabled.

Although admins won't be messing around, an attacker who phishes their way into an admin account can potentially cause a DOS by injecting code that automatically redirects users to some other website upon page load. For example, inserting this into announcements would deny access to the home page:

<script>window.location.href="http://attacker.url"</script>
jdabtieu commented 3 years ago

The fields haven't been sanitized yet, but I just pushed a bug fix that should fix the problem in your screenshot where the Markdown was polluting the rest of the page

jdabtieu commented 3 years ago

Fixed in 2ca19ff. While admins can XSS themselves on the create/edit pages (but why would anyone do that), the HTML code that finally gets generated is sanitized.