prantlf / jsonlint

JSON/CJSON/JSON5 parser, syntax & schema validator and pretty-printer with a command-line client, written in pure JavaScript.
http://prantlf.github.io/jsonlint/
MIT License
38 stars 9 forks source link

`--no-duplicate-keys` false positive when key is `constructor` #23

Closed hyperupcall closed 3 months ago

hyperupcall commented 1 year ago

Reproduction

#!/usr/bin/env sh
cat <<"EOF"
{
  "constructor": "value"
}
EOF
npx  @prantlf/jsonlint --quiet --no-duplicate-keys file.json

Output:

File: file.json
Parse error on line 2, column 18:
{    "constructor": "value"}
------------------^
Duplicate key: "constructor"

This may potentially be a prototype pollution vulnerability for constructor? Anyways, thank you for the work you have done on this package.

prantlf commented 3 months ago

Thank you, this is a very interesting found! Luckily, there's no prototype pollution, if keys constructor or __proto__ are used in an object. But there's a bug that the duplicates are checked not only in the own object, but down to the prototype. I'm going to fix it in the upcoming release.

If I use this input file:

{
  "answer": 42,
  "constructor": "value",
  "__proto__": { "polluted": true }
}

And I don't enable checking the duplicates, all keys are preserved, because that's how JSON.parse fills the object keys in the result:

❯ jsonlint test.json
{
  "answer": 42,
  "constructor": "value",
  "__proto__": {
    "polluted": true
  }
}

But if I enable the custom JSON parser by allowing comments in the input (or by other options from the JSON5 mode), the keys constructor and __proto__ are "sanitized" away:"

❯ jsonlint --comments test.json
{
  "answer": 42
}

So, somebody will say that it's great that property pollution cannot happen however the parsed output will be used, but somebody else may say that it's a data loss, skipping the special object keys. In any case, the behaviour of the custom JSON parser is different from JSON.parse. I will do something about this in the next major version.

prantlf commented 3 months ago

I made the custom parser behave just like the built-in JSON.parse - letting all prototype keys be parsed by default. If the old default behaviour is needed, the argument ignore-prototype-keys can be added to the command line, or the option ignorePrototypeKeys can be set to true, when calling the parse method.

If you don't have under control, what will happen with the parsed object, you should consider setting ignoreProtoKey to true, when calling the parse method, to prevent prototype pollution by merging the __proto__ key value to an object.

prantlf commented 3 months ago

The full change released with 15.0.0, committed in e2f8a7be3cdb78bf696ba4251e2b3053f5714c22.

The mistakenly reported constructor as a duplicate fixed in 14.1.0, committed in eb357141820cf19be227342ea944f8c3208ec865.

hyperupcall commented 3 months ago

Awesome, thank you so much!