Securely set unknown property (mitigate square bracket object injection attacks) utility function

After setting up eslint-plugin-security, I went on to attempt to address nearly 400 uses of square brackets in our javascript codebase (flagged by the rule security/detect-object-injection). Although this plugin could be a lot more intelligent, any use of square brackets could possibly be an opportunity for a malicious agent to inject their own code.

To understand how, and to understand the whole context of my question, you need to read this documentation: https://github.com/nodesecurity/eslint-plugin-security/blob/master/docs/the-dangers-of-square-bracket-notation.md

I generally tried to use Object.prototype.hasOwnProperty.call(someObject, someProperty) where I could to mitigate the chance that someProperty is maliciously set to constructor. Lot of situations were simply dereferencing an array index in for loops (for (let i=0;i<arr.length;i++) { arr[i] }) If i is always an int, this is obviously always safe.

One situation I don't think I have handled perfectly, are square bracket assignments like this:

someObject[somePropertyPotentiallyDefinedFromBackend] = someStringPotentiallyMaliciouslyDefinedString

The status quo for StackOverflow is to "show me all the code" - when you are going through a codebase and fixing these in hundreds upon hundreds of instances, it takes a lot longer to go and read the code that comes before one of these assignments. Furthermore, we want to make sure this code stays secure as it's modified in the future.

How can we make sure the property being set is essentially not already defined on vanilla objects? (i.e. constructor)

Attempted to solve this myself, but missing some pieces - see if you can get rest of unit tests to pass:

So I think the easiest way to solve this issue is with a simple util, safeKey defined as such:

// use window.safeKey = for easy tinkering in the console.
const safeKey = (() => {
  // Safely allocate plainObject's inside iife
  // Since this function may get called very frequently -
  // I think it's important to have plainObject's
  // statically defined
  const obj = {};
  const arr = [];
  // ...if for some reason you ever use square brackets on these types...
  // const fun = function() {}
  // const bol = true;
  // const num = 0;
  // const str = '';
  return key => {
    // eslint-disable-next-line security/detect-object-injection
    if (obj[key] !== undefined || arr[key] !== undefined
      // ||
      // fun[key] !== undefined ||
      // bol[key] !== undefined ||
      // num[key] !== undefined ||
      // str[key] !== undefined
    ) {
      return 'SAFE_'+key;
    } else {
      return key;
    }
  };
})();

We could also write a util safeSet - instead of this:

obj[key] = value;

You would do this:

safeSet(obj, key, value)

Tests for safeKey (failing):

console.log(safeKey('toString'));
//     Good: => SAFE_toString

console.log(safeKey('__proto__'));
//     Good: => SAFE___proto__

console.log(safeKey('constructor'));
//     Good: => SAFE_constructor

console.log(safeKey('prototype'));
//     Fail: =>      prototype

console.log(safeKey('toJSON'));
//     Fail: =>      toJSON

You'd then use it like so:

someObject[safeKey(somePropertyPotentiallyDefinedFromBackend)] = someStringPotentiallyMaliciouslyDefinedString

This means if the backend incidentally sends json with a key somewhere of constructor we don't choke on it, and instead just use the key SAFE_constructor (lol). Also applies for any other pre-defined method/property, so now the backend doesn't have to worry about json keys colliding with natively defined js properties/methods.

This util function is nothing without a series of passing unit tests. As I've commented not all the tests are passing. I'm not sure which object(s) natively define toJSON - and this means it may need to be part of a hardcoded list of method/property names that have to be blacklisted. But I'm not sure how to find out every one of these property methods that needs to be blacklisted. So we need to know the best way anyone can generate this list, and keep it updated.

I did find that using Object.freeze(Object.prototype) helps, but methods like toJSON I don't think exist on the prototype.

Here's another little test case:

const prop = 'toString';
someData[safeKey(prop)] = () => {
    alert('hacked');
    return 'foo';
};
console.log('someProp.toString()', someData + '');

from Securely set unknown property (mitigate square bracket object injection attacks) utility function