title: How can I mitigate injection/exfiltration attacks from dynamic property accesses (i.e. square bracket notation) in JavaScript?
body: <p>After setting up <a href="https://github.com/nodesecurity/eslint-plugin-security" rel="noreferrer"><code>eslint-plugin-security</code></a>, 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 uses of square brackets could possibly be an opportunity for a malicious agent to inject their own code.</p>
  <p>To understand how, and to understand the whole context of my question, you need to read this documentation: <a href="https://github.com/nodesecurity/eslint-plugin-security/blob/master/docs/the-dangers-of-square-bracket-notation.md" rel="noreferrer">https://github.com/nodesecurity/eslint-plugin-security/blob/master/docs/the-dangers-of-square-bracket-notation.md</a></p>
  <p>I generally tried to use <code>Object.prototype.hasOwnProperty.call(someObject, someProperty)</code> where I could to mitigate the chance that <code>someProperty</code> is maliciously set to <code>constructor</code>. Lot of situations were simply dereferencing an array index in for loops (<code>for (let i=0;i&lt;arr.length;i++) { arr[i] }</code>) If <code>i</code> is always a number, this is obviously always safe.</p>
  <p>One situation I don't think I have handled perfectly, are square bracket <em>assignments</em> like this:</p>
  <pre><code>someObject[somePropertyPotentiallyDefinedFromBackend] = someStringPotentiallyMaliciouslyDefinedString
  </code></pre>
  <p>I think the easiest way to solve this issue is with a simple util, <code>safeKey</code> defined as such:</p>
  <pre><code>// use window.safeKey = for easy tinkering in the console.
  const safeKey = (() =&gt; {
    // 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 =&gt; {
      // 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;
      }
    };
  })();
  </code></pre>
  <p>You'd then use it like so:</p>
  <pre><code>someObject[safeKey(somePropertyPotentiallyDefinedFromBackend)] = someStringPotentiallyMaliciouslyDefinedString
  </code></pre>
  <p>This means if the backend incidentally sends JSON with a key somewhere of <code>constructor</code> we don't choke on it, and instead just use the key <code>SAFE_constructor</code> (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.</p>
  <p>This utility 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 <code>toJSON</code> - 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 <em>every</em> 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.</p>
  <p>I did find that using <code>Object.freeze(Object.prototype)</code> helps, but I don't think methods like <code>toJSON</code> exist on the prototype.</p>
  <p>How can we make sure the property being set is essentially not already defined on vanilla objects? (i.e. <code>constructor</code>)</p>