From d4e6cb956ae51c8bb2828e71c7c1107c340cf1e8 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Fri, 5 Mar 2021 11:18:34 +0100 Subject: [PATCH] Merge pull request from GHSA-gmjw-49p4-pcfm Co-authored-by: Simone Busoli --- README.md | 1 + index.js | 4 ++- lib/decoder.js | 11 +++++++ test/object-prototype-poisoning.js | 49 ++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 test/object-prototype-poisoning.js diff --git a/README.md b/README.md index e8d5a49..448ac53 100644 --- a/README.md +++ b/README.md @@ -111,6 +111,7 @@ options: - `compatibilityMode`, a boolean that enables "compatibility mode" which doesn't use str 8 format. Defaults to false. - `disableTimestampEncoding`, a boolean that when set disables the encoding of Dates into the [timestamp extension type](https://github.com/msgpack/msgpack/blob/master/spec.md#timestamp-extension-type). Defaults to false. - `preferMap`, a boolean that forces all maps to be decoded to `Map`s rather than plain objects. This ensures that `decode(encode(new Map())) instanceof Map` and that iteration order is preserved. Defaults to false. +- `protoAction`, a string which can be `error|ignore|remove` that determines what happens when decoding a plain object with a `__proto__` property which would cause prototype poisoning. `error` (default) throws an error, `remove` removes the property, `ignore` (not recommended) allows the property, thereby causing prototype poisoning on the decoded object. ------------------------------------------------------- diff --git a/index.js b/index.js index 952c866..6557e35 100644 --- a/index.js +++ b/index.js @@ -19,7 +19,9 @@ function msgpack (options) { // if true, skips encoding Dates using the msgpack // timestamp ext format (-1) disableTimestampEncoding: false, - preferMap: false + preferMap: false, + // options.protoAction: 'error' (default) / 'remove' / 'ignore' + protoAction: 'error' } decodingTypes.set(DateCodec.type, DateCodec.decode) diff --git a/lib/decoder.js b/lib/decoder.js index 3db580a..d43d52c 100644 --- a/lib/decoder.js +++ b/lib/decoder.js @@ -187,6 +187,17 @@ module.exports = function buildDecode (decodingTypes, options) { for (let i = 0; i < 2 * length; i += 2) { const key = result[i] const val = result[i + 1] + + if (key === '__proto__') { + if (options.protoAction === 'error') { + throw new SyntaxError('Object contains forbidden prototype property') + } + + if (options.protoAction === 'remove') { + continue + } + } + object[key] = val } return [object, consumedBytes] diff --git a/test/object-prototype-poisoning.js b/test/object-prototype-poisoning.js new file mode 100644 index 0000000..18644cd --- /dev/null +++ b/test/object-prototype-poisoning.js @@ -0,0 +1,49 @@ +'use strict' + +var test = require('tape').test +var msgpack = require('../') + +test('decode throws when object has forbidden __proto__ property', function (t) { + const encoder = msgpack() + + const payload = { hello: 'world' } + Object.defineProperty(payload, '__proto__', { + value: { polluted: true }, + enumerable: true + }) + + const encoded = encoder.encode(payload) + + t.throws(() => encoder.decode(encoded), /Object contains forbidden prototype property/) + t.end() +}) + +test('decode ignores forbidden __proto__ property if protoAction is "ignore"', function (t) { + const encoder = msgpack({ protoAction: 'ignore' }) + + const payload = { hello: 'world' } + Object.defineProperty(payload, '__proto__', { + value: { polluted: true }, + enumerable: true + }) + + const decoded = encoder.decode(encoder.encode(payload)) + + t.equal(decoded.polluted, true) + t.end() +}) + +test('decode removes forbidden __proto__ property if protoAction is "remove"', function (t) { + const encoder = msgpack({ protoAction: 'remove' }) + + const payload = { hello: 'world' } + Object.defineProperty(payload, '__proto__', { + value: { polluted: true }, + enumerable: true + }) + + const decoded = encoder.decode(encoder.encode(payload)) + + t.equal(decoded.polluted, undefined) + t.end() +})