XSS protection - hook on runtime vnode patching?

[OpenGG]

What problem does this feature solve?

The following code is vulnerable to XSS attacks:

<a :href="untrusted_data">

<p v-html="untrusted_data"></p>

<img :onload="untrusted_data" :onerror="untrusted_data">

Web devs can do filtering in application level, via custom filters. However there are problems too:

1. Application code could have no knowledge of runtime, is it running in browsers/weex/... ?
2. There might be human errors, e.g. a newcomer of dev group fails to apply proper filter to an untrusted field.

What really interests me is xss filtering in renderer or vnode level, while being kept transparent to application code.

What does the proposed API look like?

Vue.vnodeHooks.prepatch(vnode => {}), called before patching, for every vnode.

const getAttribute = (vnode, attr) => {
  const val = vnode.data && vnode.data.attrs && vnode.data.attrs[attr]
  return val
}

const setAttribute = (vnode, attr, val) => {
  vnode.data.attrs[attr] = val
}

const getInnerHTML = (vnode) => {
  const html = vnode.data && vnode.data.domProps && vnode.data.domProps.innerHTML
  return html
}

const setInnerHTML = (vnode, html) {
  vnode.data.domProps.innerHTML = html
}

// called before patching for every vnode
Vue.vnodeHooks.prepatch((vnode) => {
  // ignore components
  if (vnode.componentInstance) {
    return
  }

  // remove onload/onerror string
  const onload = getAttribute(vnode, 'onload')
  const onerror = getAttribute(vnode, 'onerror')
  if (typeof onload === 'string') {
    setAttribute('onload', '')
  } else if (typeof onerror === 'string') {
    setAttribute('onerror', '')
  }

  // redirect javascript href to 404
  if (vnode.tag === 'a') {
    const href = getAttribute(vnode, 'href')
    if (typeof href === 'string' && /^\s*javascript:/i.test(href)) {
      setAttribute(vnode, 'href', '/404')
    }
  }

  // sanitize html
  const html = getInnerHTML(vnode)
  if (typeof html === 'string') {
    setInnerHTML(
      vnode,
      sanitize(html)
    )
  }
})

[posva]

eem, I'm not sure about this.

• <a :href="untrusted_data"> can only happen if you have some kind of website with content completely provided by the users and set the whole href attribute (instead of :href="'/redirect=' + encode(url)" or similar), in which case it makes sense to sanitize some of the data. In most applications, one would never write a link with the whole url coming from non-verified user content. So adding some special handling would have a negative impact on performance for a very specific use case that most people do not need.
• <p v-html="untrusted_data"></p>: this is developer responsibility as we say in the docs https://vuejs.org/v2/api/#v-html
• <img :onload="untrusted_data" :onerror="untrusted_data"> This is pretty much explicitly writing code that leads to XSS. I don't see why anyone would write this code besides testing purposes

What I think would make more sense is to either add an entry to the cookbook or a page about XSS in the docs cc @chrisvfritz @sdras

[OpenGG]

@posva

Agreed on the possible performance impact. I will try to do the xss filter check in compile phase.

Meanwhile, more detailed XSS explanation in docs could be great.

[chrisvfritz]

@posva @OpenGG I'm working on a security page as we speak. 🙂

[chrisvfritz]

Btw, this is what I have so far in our upcoming security guide regarding this specific issue:

In a URL like this:

<a v-bind:href="userProvidedUrl">
  click me
</a>

There's a potential security issue if the URL has not been "sanitized" to prevent JavaScript execution using javascript:. There are libraries such as sanitize-url to help with this, but note:

If you're ever doing URL sanitization on the frontend, you already have a security issue. User-provided URLs should always be sanitized by your backend before even being saved to a database. Then the problem is avoided for _every_ client connecting to your API, including native mobile apps.

Unfortunately, Vue also cannot help you guarantee that user-provided URLs lead to safe destinations.

Feedback welcome. 🙂

[OpenGG]

@chrisvfritz

Mention :onload="string" :onerror="string" :onclick="string" maybe?

Also, it just occur to me: if security checks may impact performance, is it possible to do that checking in development mode, warning people about these potential xss vulnerabilities when detected?

[posva]

It's not really possible to add a warning in development mode as we cannot know if the data is untrusted. So we could only warn if we find something like javascript: in the data which means that the developer is already aware or will see the vulnerability themselves.
Closing since Chris is working on some documentation for this

[chrisvfritz]

@OpenGG I do mention event attributes like that. 🙂