Merge pull request #25 from fastify/improve-contractor-detection

Throw only if the constructor key has a child named prototype

Author: delvedor

README.md

@@ -38,7 +38,7 @@ npm install secure-json-parse
 ## Usage
 
 Pass the option object as a second (or third) parameter for configuring the action to take in case of a bad JSON, if nothing is configured, the default is to throw a `SyntaxError`.<br/>
-You can choose which action to perform in case `__proto__` is present, and in case `constructor` is present.
+You can choose which action to perform in case `__proto__` is present, and in case `constructor.prototype` is present.
 
 ```js
 const sjson = require('secure-json-parse')
@@ -63,7 +63,7 @@ Parses a given JSON-formatted text into an object where:
         - `'remove'` - deletes any `__proto__` keys from the result object.
         - `'ignore'` - skips all validation (same as calling `JSON.parse()` directly).
     - `constructorAction` - optional string with one of:
-        - `'error'` - throw a `SyntaxError` when a `constructor` key is found. This is the default value.
+        - `'error'` - throw a `SyntaxError` when a `constructor.prototype` key is found. This is the default value.
         - `'remove'` - deletes any `constructor` keys from the result object.
         - `'ignore'` - skips all validation (same as calling `JSON.parse()` directly).
 
@@ -76,7 +76,7 @@ Scans a given object for prototype properties where:
         - `'error'` - throw a `SyntaxError` when a `__proto__` key is found. This is the default value.
         - `'remove'` - deletes any `__proto__` keys from the input `obj`.
     - `constructorAction` - optional string with one of:
-        - `'error'` - throw a `SyntaxError` when a `constructor` key is found. This is the default value.
+        - `'error'` - throw a `SyntaxError` when a `constructor.prototype` key is found. This is the default value.
         - `'remove'` - deletes any `constructor` keys from the input `obj`.
 
 ## Benchmarks

index.js

@@ -76,7 +76,9 @@ function scan (obj, { protoAction = 'error', constructorAction = 'error' } = {})
         delete node.__proto__ // eslint-disable-line no-proto
       }
 
-      if (constructorAction !== 'ignore' && Object.prototype.hasOwnProperty.call(node, 'constructor')) { // Avoid calling node.hasOwnProperty directly
+      if (constructorAction !== 'ignore' &&
+          Object.prototype.hasOwnProperty.call(node, 'constructor') &&
+          Object.prototype.hasOwnProperty.call(node.constructor, 'prototype')) { // Avoid calling node.hasOwnProperty directly
         if (constructorAction === 'error') {
           throw new SyntaxError('Object contains forbidden prototype property')
         }

test.js

@@ -185,6 +185,14 @@ test('parse', t => {
       t.end()
     })
 
+    t.test('sanitizes object string (no prototype key)', t => {
+      t.deepEqual(
+        j.parse('{"a": 5, "b": 6,"constructor":{"bar":"baz"} }', { constructorAction: 'remove' }),
+        { a: 5, b: 6, constructor: { bar: 'baz' } }
+      )
+      t.end()
+    })
+
     t.test('sanitizes nested object string', t => {
       t.deepEqual(
         j.parse('{ "a": 5, "b": 6, "constructor":{"prototype":{"bar":"baz"}}, "c": { "d": 0, "e": "text", "constructor":{"prototype":{"bar":"baz"}}, "f": { "g": 2 } } }', { constructorAction: 'remove' }),
@@ -217,6 +225,11 @@ test('parse', t => {
       t.end()
     })
 
+    t.test('Should not throw if the constructor key hasn\'t a child named prototype', t => {
+      t.doesNotThrow(() => j.parse('{ "a": 5, "b": 6, "constructor":{"bar":"baz"} }', null, null), SyntaxError)
+      t.end()
+    })
+
     t.test('errors on proto property (null, null)', t => {
       t.throws(() => j.parse('{ "a": 5, "b": 6, "constructor":{"prototype":{"bar":"baz"}} }', null, null), SyntaxError)
       t.end()