Group-income-simple: Check the project for XSS [$100 bounty]

[taoeffect]

Problem

There are several places in the project where user-provided data is combined with v-html, and all such cases are dangerous if not sanitized properly.

For example, in #934 we need to make sure that stuff like this doesn't lead to any problems:

    body: L('The group has a new member. Say hi to {name}!', {
      name: `<strong>${name}</strong>`
    }),

And later on we have:

              span.c-item-text(v-html='item.body')

If name is a username containing HTML or allowing dangerous escapes, this could potentially be a problem. We use v-html to support formatting in strings (e.g. <strong>), but we don't want that to introduce security vulnerabilities.

Solution

Combe the project for all such potential vulnerabilities. Test to see if they actually exist. If they do, fix all of them and come up with a generalized way of prevent all such instances from ever happening again. We should not have to police our PRs for this, we should have an automated way of dealing with it.

EDIT: use v-safe-html and v-safe-link to address this issue.

Bounty

$100 bounty for a clean solution to this (paid in cryptocurrency).

[snowteamer]

For me using v-html is a bit like using eval - I would rather avoid using it entirely if possible.
At least I think it should not be used with anything dynamic like variable or template literals.

If we only need this directive to render simple presentational markup like <strong>, maybe we could define our own custom Vue directive which would not use innerHTML internally, and replace every occurence of v-html with it.

For example, something like v-simple-html.

[taoeffect]

Yes, that sounds like a great idea @snowteamer! v-simple-html, or better yet, v-safe-html, could have a whitelist of allowed HTML tags and attributes and still use innerHTML.

We still need to make sure though that it works well, because if there's a mistake then some bad code could still get through. We could do an inventory of what tags and attributes are used in the project, and only allow them.

Note: when I say, "and attributes", I mean we need to make sure that if we whitelist <strong> that we do not allow <strong onclick=" .. XSS .."> or anything of the sort. The most I've seen is using class and style, so those would probably be the only two attributes we'd allow.

[taoeffect]

Hah! Just noticed that Gitlab created a v-safe-html directive that uses DOMPurify under the hood! Just what we need to close this issue!

• https://docs.gitlab.com/ee/development/secure_coding_guidelines.html#xss-mitigation-and-prevention-in-javascript-and-vue
• https://gitlab-org.gitlab.io/gitlab-ui/?path=/story/directives-safe-html-directive--default

And here are direct links to the very useful v-safe-html and v-safe-link directives for closing this issue.

[taoeffect]

Adding a $100 bounty for closing this issue for non-contractors (meaning, if you don't have a development contract with okTurtles, and you'd like to close this issue, you can work on it, and if we accept your PR, you'll receive the bounty).

Anyone else can add to the bounty too if they'd like by following these simple steps:

1. Donate the amount you'd like to dedicate to this issue to the okTurtles Foundation.
2. Make a comment here (or privately to us via Gitter or Slack) the same day letting us know you'd like your donation to be used for a bounty to close an issue, and which issue that is.