#236 Not Completely Fixed

[AmazingMech2418]

#236 explains a vulnerability in processNested that could result in a DoS attack. While prototype pollution specifically is fixed, you can still modify a method of an object or array.

Let's say you are using the toString method of the object returned by parseNested...

const processNested = require('./processNested.js'); // assume that that is the correct path to the module.

let obj = processNested({
  "toString": null
});
console.log(obj);
console.log(obj.toString());

This will return the error

TypeError: obj.toString is not a function
    at <insert path here>/index.js:7:17
    at Script.runInContext (vm.js:131:20)
    at Object.<anonymous> (/run_dir/interp.js:156:20)
    at Module._compile (internal/modules/cjs/loader.js:1138:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1158:10)
    at Module.load (internal/modules/cjs/loader.js:986:32)
    at Function.Module._load (internal/modules/cjs/loader.js:879:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)

However, the toString method is not commonly used in plain objects since it just returns [object Object]. However, this vulnerability extends to arrays.

Let's say you have

const processNested = require('./processNested.js'); // assume that that is the correct path to the module.

let obj = processNested({
  "array[0]": 1, // The function needs to read this as an array, so we have a random value there for the main array.
  "array[join]": null // This modifies the Array.join method.
});
console.log(obj);
console.log(obj.array.join());

This returns the error

TypeError: obj.array.join is not a function
    at <insert path here>/index.js:8:23
    at Script.runInContext (vm.js:131:20)
    at Object.<anonymous> (/run_dir/interp.js:156:20)
    at Module._compile (internal/modules/cjs/loader.js:1138:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1158:10)
    at Module.load (internal/modules/cjs/loader.js:986:32)
    at Function.Module._load (internal/modules/cjs/loader.js:879:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)

which, if not escaped properly, could result in a DoS attack by the error being thrown in the server code and the code being exited, shutting down the server. Of course, this isn't a hugely critical issue since the code could be escaped using a try...catch statement, but this is not an expected error as it occurs within the module and may not be tested, so a developer may not think to escape the code to catch errors since no error would be expected.

Additionally, someone could potentially modify a method to execute arbitrary code that could cause malware to be inserted in the system, or even possibly a remote shell to be connected to a server. I'm not sure how great of a risk this is, however, since I do not think JSON can allow the transfer of functions.

This may not have been a huge issue if it weren't for the initial issue being opened and exposing the prototype pollution initially and it seems like a "quick fix" was used instead of completely fixing the bug.

I would recommend using Object.getOwnPropertyNames on the prototypes of Object and Array to define the invalid keys instead of using the current method of using a static array to do so.