[ckan-changes] [ckan/ckan] 68fe12: [#1792] Fix XSS and bug with numeric/quoted values...

[GitHub]

  Branch: refs/heads/1792-filterable-resource-views
  Home:   https://github.com/ckan/ckan
  Commit: 68fe1248370ee1d5bcb2c5d6cb1ec41272d5a747
      https://github.com/ckan/ckan/commit/68fe1248370ee1d5bcb2c5d6cb1ec41272d5a747
  Author: Vitor Baptista <vitor at vitorbaptista.com>
  Date:   2014-08-05 (Tue, 05 Aug 2014)

  Changed paths:
    M ckan/public/base/javascript/view-filters.js
    M ckan/public/base/test/spec/view-filters.spec.js

  Log Message:
  -----------
  [#1792] Fix XSS and bug with numeric/quoted values in filters

The problem is with the function `queryStringToJSON` that we use in
`ckan/public/base/javascript/view-filters.js` to convert the URL's query string
into a JavaScript object to then parse it.

To understand the bug, take a look at this example:

```javascript
> "filters=country:Brazil|year:2014".queryStringToJSON()
Object {filters: "country:Brazil|year:2014"}
> "filters=country:Brazil".queryStringToJSON()
Object {filters: "country:Brazil"}
> "filters=year:2014".queryStringToJSON()
Object {filters: 2014} // The correct result would be { filters: "year:2014" }
```

Looking at `queryStringToJSON` code, I found the problematic part at
https://github.com/ckan/ckan/blob/1792-filterable-resource-views/ckan/public/base/javascript/view-filters.js#L49-L57

```javascript
// ...

// Fix
key = decodeURIComponent(key);
value = decodeURIComponent(value);
try {
  // value can be converted
  value = eval(value);
} catch ( e ) {
  // value is a normal string
}

// ...
```

This code tries to `eval` the query string's values. "Normal" strings throw an
error when eval'd, which the code ignores. But unfortunately `"year:2014"`
isn't a normal string. See:

```javascript
> eval("year:2014")
2014
> year: 2014
2014
> { year: 2014 }
2014
> eval("year:2014|country:Brazil")
SyntaxError: Unexpected token :
> eval("country:Brazil")
ReferenceError: Brazil is not defined
```

We'll have the same problem if the filter value is between quotes, as in:

```javascript
> "filters=country:'Brazil'".queryStringToJSON()
Object {filters: "Brazil"}
```

The XSS issue is related to us calling `eval()` on all parameters, so some
malicious user could send a link to e.g.
`http://demo.ckan.org/?param=alert('abc')` and execute JS code on anyone that
clicks on that. This was discovered in https://github.com/balupton/jquery-sparkle/pull/5