swordev / merge Goto Github PK

A Prototype pollution attack caused by merge 1.2.0 with High rank has been disclosed recently at https://hackerone.com/reports/381194

Please see if it can be fixed

The code examples in the readme are not self-explanatory.
Some description of the API would be nice.

I'm a member of the Node.js Ecosystem Security Working Group and we received a report of a vulnerability in this module through HackerOne.

We tried inviting the author by e-mail but received no response so I'm opening this issue and inviting anyone with commit and npm publish rights to collaborate with us on a fix.

I just got an issue with a Map resulting in an empty element :(

This will reduce the size of the package

Doesn't work for me.

/var/www/nodeserver/node_modules/merge/index.js:12
                                Object.defineProperty(y, w, Object.getOwnPropertyDescriptor(z, w));
                                       ^
TypeError: Object.defineProperty called on non-object

In absence of a security policy I am creating the issue here.
CVE-2020-28499
https://nvd.nist.gov/vuln/detail/CVE-2020-28499#match-6281551

Two objects:
var one = {
meta : {
source:"One"
}
}

var two = {
meta : {
source2:"Two"
}
}

var newData = merge(one, two)

and the result is:
{
"meta": {
"source2": "Two"
}
}

I would have expected:
{
"meta": {
"source": "One",
"source2": "Two"
}
}

am i off base here? Or using it incorrectly?

Thanks

yeikos. Sorry to trouble you. Can you add a license file for this package? I can not use it without license.

The for ... in loop in this module should only copy a property if item.hasOwnProperty(key), otherwise it will copy prototype properties, resulting in unexpected and hard-to-find bugs.

Hi,

I'd be great if you add a typescript definitions file. I've created one for you already.

declare module 'merge' {
  function merge<A extends Object>(a: A): A;
  function merge<A extends Object, B>(a: A, b: B): A & B;
  function merge<A extends Object, B, C>(a: A, b: B, c: C): A & B & C;
  function merge<A extends Object, B, C, D>(a: A, b: B, c: C, d: D): A & B & C & D;
  function merge<A extends Object, B, C, D, E>(a: A, b: B, c: C, d: D, e: E): A & B & C & D & E;

  function merge<A extends Object>(recursive: boolean, a: A): A;
  function merge<A extends Object, B>(recursive: boolean, a: A, b: B): A & B;
  function merge<A extends Object, B, C>(recursive: boolean, a: A, b: B, c: C): A & B & C;
  function merge<A extends Object, B, C, D>(recursive: boolean, a: A, b: B, c: C, d: D): A & B & C & D;
  function merge<A extends Object, B, C, D, E>(recursive: boolean, a: A, b: B, c: C, d: D, e: E): A & B & C & D & E;

  namespace merge {
    function recursive<A extends Object>(a: A): A;
    function recursive<A extends Object, B>(a: A, b: B): A & B;
    function recursive<A extends Object, B, C>(a: A, b: B, c: C): A & B & C;
    function recursive<A extends Object, B, C, D>(a: A, b: B, c: C, d: D): A & B & C & D;
    function recursive<A extends Object, B, C, D, E>(a: A, b: B, c: C, d: D, e: E): A & B & C & D & E;
  }

  export = merge;
}

You'd make my life much easier :)

Please consider the following code:

var a = { "debug" : true, "hagemashi" : "yasashii", "okuritai" : {
        "koto" : "ichi",
        "mono" : "ni"
    }
};
console.log( a );
var b = { "debug" : false, "hagemashi" : "yasashikunai", "okuritai" : {
        "koto" : "san",
        "mono" : "shi"
    }
};
console.log( merge.recursive( false, a, b ) );

console.log( a );

Outputs

{ debug: true,
  hagemashi: 'yasashii',
  okuritai: { koto: 'ichi', mono: 'ni' } }
{ debug: false,
  hagemashi: 'yasashikunai',
  okuritai: { koto: 'san', mono: 'shi' } }
{ debug: true,
  hagemashi: 'yasashii',
  okuritai: { koto: 'san', mono: 'shi' } }

I was assuming the last two outputs would be identical since the intention is to modify the original object. Am I using it incorrectly?

Thanks for sharing merge. When upgrading from v1 to v2 I found the following bug. Bit confused as what could have caused it as the clone code doesn't seem to have changed greatly (other than being converted to TypeScript)

const merge = require('merge');
const result = merge.recursive(true, { regex: /.*/ } , {});
console.log(result);

v1.x

{ regex: /.*/ }

v2.x

{ regex: {} }

Hi,

Could you patch this? See links below:

https://hackerone.com/reports/381194
https://nvd.nist.gov/vuln/detail/CVE-2018-16469
jestjs/jest#7318 (comment)

Great library. Would be nice to have ES6 Module support

There should be .npmignore file excluding tests directory from being published to npm.

Otherwise every sublibrary library in my project that depends on merge pulls additional 1.3MB of useless (in production) files.

.npmignore

  tests

Hi,
There is a new security issue : https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-28499
is there a plan to resolve this?
thanks,
Tal

The homepage link http://www.yeikos.com is down at the moment.

Hi, here is this critical vulnerability that affects this project.
CVE-2021-23397
https://nvd.nist.gov/vuln/detail/CVE-2021-23397

Although I know this vulnerability is about another library but that came up running the dependency check over this merge library!

Suppose I have the following:

a = {"_id": "1", "name": "marcello"}
b = {"_id": "1", "country": "US"}

merge(a, b) = {"_id": "1", "name": "marcello", "_id": "1", "country": "US"}...

The two ID properties were not "merged" into the same document... Is this the intended behavior?

If so, is there a way to extend the command to accept either the right or the left object?

merge(a, b, L) = {"_id": "1", "name": "marcello", "country": "US"}...

a = {"_id": "1", "name": "marcello"}
b = {"_id": "1", "country": "US", "name":"Marcello de Sales", "state": "CA"}

merge(a, b, R) = {"_id": "1", "name": "Marcello de Sales", "country": "US", "state": "CA"}...

That way, key collisions could be treated as a matter of the same property, replacing the proper side of the merge...

thanks!

Didn't see the necessary for this repo because in NodeJs Object.assign() can do the same thing.

var merge = require("merge");
var A = {
    x: 1,
    y: 2
    };
var B = {
    z: 3
    };

 console.log(A)
 C = merge(A, B);
 console.log(A);

yields

{ x: 1,
  y: 2,
  z: 3 }

console.log(merge.recursive({'one':['a','b']},{'one':['c','d']}))
=> { one: [ 'c', 'd' ] }

I expected:

=> { one: ['a','b','c','d'] }

In the meantime, it looks like deepmerge does this.

Symbols can be used as property keys, but they are ignored.

const merge = require('merge');

const mySymbol = Symbol('mySymbol');

const x = { value: 42, [mySymbol]: 'hello' };

console.log(x);
// { value: 42, [Symbol(mySymbol)]: 'hello' }

const y = { other: 33 };
const z = merge(true, x, y);

console.log(z);
// { value: 42, other: 33 }
// expected: { value: 42, other: 33, [Symbol(mySymbol)]: 'hello' }

When merging two objects a undefined property overrides a present property.
I think a better solution would be to ignore undefined properties, since other libraries like underscore handle it this way.

I would like to run npx merge ./foo.json ./bar.json or similar from my project build scripts.