Repository: elastic/elasticsearch-js
Pull request #1420 (2021-03-15 07:52:05)

[Backport 7.x] Disable prototype poisoning option
Backport 6a30cd995513e44354f9b5fcb75161ddd3fe054c from #1414

elasticmachine: Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?


Patch:
diff --git a/docs/basic-config.asciidoc b/docs/basic-config.asciidoc
index a6a4b41d..38ed7c1d 100644
--- a/docs/basic-config.asciidoc
+++ b/docs/basic-config.asciidoc
@@ -244,4 +244,8 @@ const client = new Client({
 })
 ----
 
+|`disablePrototypePoisoningProtection`
+|`boolean`, `'proto'`, `'constructor'` - By the default the client will protect you against prototype poisoning attacks. Read https://web.archive.org/web/20200319091159/https://hueniverse.com/square-brackets-are-the-enemy-ff5b9fd8a3e8?gi=184a27ee2a08[this article] to learn more. If needed you can disable prototype poisoning protection entirely or one of the two checks. Read the `secure-json-parse` https://github.com/fastify/secure-json-parse[documentation] to learn more. +
+_Default:_ `false`
+
 |===
\ No newline at end of file
diff --git a/index.d.ts b/index.d.ts
index 0eca01c9..5606a7cf 100644
--- a/index.d.ts
+++ b/index.d.ts
@@ -114,7 +114,8 @@ interface ClientOptions {
     // TODO: remove username and password here in 8
     username?: string;
     password?: string;
-  }
+  };
+  disablePrototypePoisoningProtection?: boolean | 'proto' | 'constructor';
 }
 
 declare class Client {
diff --git a/index.js b/index.js
index 76b517a1..02851266 100644
--- a/index.js
+++ b/index.js
@@ -123,7 +123,8 @@ class Client extends ESAPI {
         opaqueIdPrefix: null,
         context: null,
         proxy: null,
-        enableMetaHeader: true
+        enableMetaHeader: true,
+        disablePrototypePoisoningProtection: false
       }, opts)
 
     this[kInitialOptions] = options
@@ -140,7 +141,9 @@ class Client extends ESAPI {
       this[kEventEmitter] = options[kChild].eventEmitter
     } else {
       this[kEventEmitter] = new EventEmitter()
-      this.serializer = new options.Serializer()
+      this.serializer = new options.Serializer({
+        disablePrototypePoisoningProtection: options.disablePrototypePoisoningProtection
+      })
       this.connectionPool = new options.ConnectionPool({
         pingTimeout: options.pingTimeout,
         resurrectStrategy: options.resurrectStrategy,
diff --git a/lib/Helpers.js b/lib/Helpers.js
index b5fc9e91..184e256b 100644
--- a/lib/Helpers.js
+++ b/lib/Helpers.js
@@ -421,7 +421,7 @@ class Helpers {
    */
   bulk (options, reqOptions = {}) {
     const client = this[kClient]
-    const { serialize, deserialize } = client.serializer
+    const { serializer } = client
     if (this[kMetaHeader] !== null) {
       reqOptions.headers = reqOptions.headers || {}
       reqOptions.headers['x-elastic-client-meta'] = this[kMetaHeader] + ',h=bp'
@@ -505,19 +505,19 @@ class Helpers {
           ? Object.keys(action[0])[0]
           : Object.keys(action)[0]
         if (operation === 'index' || operation === 'create') {
-          actionBody = serialize(action)
-          payloadBody = typeof chunk === 'string' ? chunk : serialize(chunk)
+          actionBody = serializer.serialize(action)
+          payloadBody = typeof chunk === 'string' ? chunk : serializer.serialize(chunk)
           chunkBytes += Buffer.byteLength(actionBody) + Buffer.byteLength(payloadBody)
           bulkBody.push(actionBody, payloadBody)
         } else if (operation === 'update') {
-          actionBody = serialize(action[0])
+          actionBody = serializer.serialize(action[0])
           payloadBody = typeof chunk === 'string'
             ? `{"doc":${chunk}}`
-            : serialize({ doc: chunk, ...action[1] })
+            : serializer.serialize({ doc: chunk, ...action[1] })
           chunkBytes += Buffer.byteLength(actionBody) + Buffer.byteLength(payloadBody)
           bulkBody.push(actionBody, payloadBody)
         } else if (operation === 'delete') {
-          actionBody = serialize(action)
+          actionBody = serializer.serialize(action)
           chunkBytes += Buffer.byteLength(actionBody)
           bulkBody.push(actionBody)
         } else {
@@ -669,13 +669,13 @@ class Helpers {
             return
           }
           for (let i = 0, len = bulkBody.length; i < len; i = i + 2) {
-            const operation = Object.keys(deserialize(bulkBody[i]))[0]
+            const operation = Object.keys(serializer.deserialize(bulkBody[i]))[0]
             onDrop({
               status: 429,
               error: null,
-              operation: deserialize(bulkBody[i]),
+              operation: serializer.deserialize(bulkBody[i]),
               document: operation !== 'delete'
-                ? deserialize(bulkBody[i + 1])
+                ? serializer.deserialize(bulkBody[i + 1])
                 /* istanbul ignore next */
                 : null,
               retried: isRetrying
@@ -716,9 +716,9 @@ class Helpers {
                 onDrop({
                   status: status,
                   error: action[operation].error,
-                  operation: deserialize(bulkBody[indexSlice]),
+                  operation: serializer.deserialize(bulkBody[indexSlice]),
                   document: operation !== 'delete'
-                    ? deserialize(bulkBody[indexSlice + 1])
+                    ? serializer.deserialize(bulkBody[indexSlice + 1])
                     : null,
                   retried: isRetrying
                 })
diff --git a/lib/Serializer.d.ts b/lib/Serializer.d.ts
index bf8453ab..7aad1d11 100644
--- a/lib/Serializer.d.ts
+++ b/lib/Serializer.d.ts
@@ -17,7 +17,12 @@
  * under the License.
  */
 
+export interface SerializerOptions {
+  disablePrototypePoisoningProtection: boolean | 'proto' | 'constructor'
+}
+
 export default class Serializer {
+  constructor (opts?: SerializerOptions)
   serialize(object: any): string;
   deserialize(json: string): any;
   ndserialize(array: any[]): string;
diff --git a/lib/Serializer.js b/lib/Serializer.js
index 8aa3d9e7..7797f1f0 100644
--- a/lib/Serializer.js
+++ b/lib/Serializer.js
@@ -23,8 +23,17 @@ const { stringify } = require('querystring')
 const debug = require('debug')('elasticsearch')
 const sjson = require('secure-json-parse')
 const { SerializationError, DeserializationError } = require('./errors')
+const kJsonOptions = Symbol('secure json parse options')
 
 class Serializer {
+  constructor (opts = {}) {
+    const disable = opts.disablePrototypePoisoningProtection
+    this[kJsonOptions] = {
+      protoAction: disable === true || disable === 'proto' ? 'ignore' : 'error',
+      constructorAction: disable === true || disable === 'constructor' ? 'ignore' : 'error'
+    }
+  }
+
   serialize (object) {
     debug('Serializing', object)
     let json
@@ -40,7 +49,7 @@ class Serializer {
     debug('Deserializing', json)
     let object
     try {
-      object = sjson.parse(json)
+      object = sjson.parse(json, this[kJsonOptions])
     } catch (err) {
       throw new DeserializationError(err.message, json)
     }


Test patch:
diff --git a/test/types/client-options.test-d.ts b/test/types/client-options.test-d.ts
index a99d0465..5a40c14b 100644
--- a/test/types/client-options.test-d.ts
+++ b/test/types/client-options.test-d.ts
@@ -557,22 +557,6 @@ expectError<errors.ConfigurationError>(
   )
 }
 
-{
-  class CustomSerializer {
-    deserialize (str: string) {
-      return JSON.parse(str)
-    }
-  }
-
-  expectError<errors.ConfigurationError>(
-    // @ts-expect-error
-    new Client({
-      node: 'http://localhost:9200',
-      Serializer: CustomSerializer
-    })
-  )
-}
-
 /**
  * `Connection` option
  */
diff --git a/test/unit/client.test.js b/test/unit/client.test.js
index be61e1bf..a0a5c5d7 100644
--- a/test/unit/client.test.js
+++ b/test/unit/client.test.js
@@ -1364,3 +1364,60 @@ test('Meta header disabled', t => {
     t.error(err)
   })
 })
+
+test('Prototype poisoning protection enabled by default', t => {
+  t.plan(1)
+
+  class MockConnection extends Connection {
+    request (params, callback) {
+      const stream = intoStream('{"__proto__":{"foo":"bar"}}')
+      stream.statusCode = 200
+      stream.headers = {
+        'content-type': 'application/json;utf=8',
+        'content-length': '27',
+        connection: 'keep-alive',
+        date: new Date().toISOString()
+      }
+      process.nextTick(callback, null, stream)
+      return { abort () {} }
+    }
+  }
+
+  const client = new Client({
+    node: 'http://localhost:9200',
+    Connection: MockConnection
+  })
+
+  client.info((err, result) => {
+    t.true(err instanceof errors.DeserializationError)
+  })
+})
+
+test('Disable prototype poisoning protection', t => {
+  t.plan(1)
+
+  class MockConnection extends Connection {
+    request (params, callback) {
+      const stream = intoStream('{"__proto__":{"foo":"bar"}}')
+      stream.statusCode = 200
+      stream.headers = {
+        'content-type': 'application/json;utf=8',
+        'content-length': '27',
+        connection: 'keep-alive',
+        date: new Date().toISOString()
+      }
+      process.nextTick(callback, null, stream)
+      return { abort () {} }
+    }
+  }
+
+  const client = new Client({
+    node: 'http://localhost:9200',
+    Connection: MockConnection,
+    disablePrototypePoisoningProtection: true
+  })
+
+  client.info((err, result) => {
+    t.error(err)
+  })
+})
diff --git a/test/unit/serializer.test.js b/test/unit/serializer.test.js
index cce887e7..d0e84730 100644
--- a/test/unit/serializer.test.js
+++ b/test/unit/serializer.test.js
@@ -157,3 +157,75 @@ test('DeserializationError', t => {
     t.ok(err instanceof DeserializationError)
   }
 })
+
+test('prototype poisoning protection', t => {
+  t.plan(2)
+  const s = new Serializer()
+  try {
+    s.deserialize('{"__proto__":{"foo":"bar"}}')
+    t.fail('Should fail')
+  } catch (err) {
+    t.ok(err instanceof DeserializationError)
+  }
+
+  try {
+    s.deserialize('{"constructor":{"prototype":{"foo":"bar"}}}')
+    t.fail('Should fail')
+  } catch (err) {
+    t.ok(err instanceof DeserializationError)
+  }
+})
+
+test('disable prototype poisoning protection', t => {
+  t.plan(2)
+  const s = new Serializer({ disablePrototypePoisoningProtection: true })
+  try {
+    s.deserialize('{"__proto__":{"foo":"bar"}}')
+    t.pass('Should not fail')
+  } catch (err) {
+    t.fail(err)
+  }
+
+  try {
+    s.deserialize('{"constructor":{"prototype":{"foo":"bar"}}}')
+    t.pass('Should not fail')
+  } catch (err) {
+    t.fail(err)
+  }
+})
+
+test('disable prototype poisoning protection only for proto', t => {
+  t.plan(2)
+  const s = new Serializer({ disablePrototypePoisoningProtection: 'proto' })
+  try {
+    s.deserialize('{"__proto__":{"foo":"bar"}}')
+    t.pass('Should not fail')
+  } catch (err) {
+    t.fail(err)
+  }
+
+  try {
+    s.deserialize('{"constructor":{"prototype":{"foo":"bar"}}}')
+    t.fail('Should fail')
+  } catch (err) {
+    t.ok(err instanceof DeserializationError)
+  }
+})
+
+test('disable prototype poisoning protection only for constructor', t => {
+  t.plan(2)
+  const s = new Serializer({ disablePrototypePoisoningProtection: 'constructor' })
+  try {
+    s.deserialize('{"__proto__":{"foo":"bar"}}')
+    t.fail('Should fail')
+  } catch (err) {
+    t.ok(err instanceof DeserializationError)
+  }
+
+  try {
+    s.deserialize('{"constructor":{"prototype":{"foo":"bar"}}}')
+    t.pass('Should not fail')
+  } catch (err) {
+    t.fail(err)
+  }
+})