javascript - Why is it bad pratice calling an array index with a variable?

Question

I'm currently developing a little game in Javascript and I'm using Codacy to review my code and help me cleaning it.

One of the most seen error is Generic Object Injection Sink (security/detect-object-injection).

It happens when I'm trying to access a value in an array using a variable. Like in this example :

function getValString(value)
{
    var values = ["Mis&eacuterable", "Acceptable", "Excellente", "Divine"];
    return values[value];
}

This function is used to display on screen the value's string of an item. It receives a "value" which can be 0, 1, 2 or 3 and returns the string of the value.

Now here's my problem :

Codacy is telling me that use of var[var] should be prohibited because it causes security issues and since I'm rather new to Javascript, I was wondering why and what are the good practices in that kind of situation.

Answer 1

The security issue present here is that the stringified value of value may be accessing a property that is inherited from the Object's __proto__ hierarchical prototype, and not an actual property of the object itself.

For example, consider the scenario when value is a string literal of "constructor".

const property = "constructor";
const object = [];
const value = object[property];

The result of value in this context will resolve to the Array() function - which is inherited as part of the Object's prototype, not an actual property of the object variable. Furthermore, the object being accessed may have overridden any of the default inherited Object.prototype properties, potentially for malicious purposes.

---

This behavior can be partially prevented by doing a object.hasOwnProperty(property) conditional check to ensure the object actually has this property. For example:

const property = "constructor";
const object = [];
if (object.hasOwnProperty(property)) {
    const value = object[property];
}

Note that if we suspect the object being accessed might be malicious or overridden the hasOwnProperty method, it may be necessary to use the Object hasOwnProperty inherited from the prototype directly: Object.prototype.hasOwnProperty.call(object, property)
Of course, this assumes that our Object.prototype has not already been tampered with.

This is not necessarily the full picture, but it does demonstrate a point.

---

Check out the following resources which elaborates in more detail why this is an issue and some alternative solutions:

• https://github.com/nodesecurity/eslint-plugin-security/blob/master/docs/the-dangers-of-square-bracket-notation.md
• Securely set unknown property (mitigate square bracket object injection attacks) utility function