11

I'm trying to read a JSON array. Every time i try to read the array/value by passing JSON object key like this-

json[key]

It shows a Eslint error-

[eslint] Generic Object Injection Sink (security/detect-object-injection)

I understand its a security warning because the key may not exists. But how do i resolve this warning? Is there any easier way to read the Json object. My plan is to pass the "key" to the function and read the json based on the key.

saz
  • 955
  • 5
  • 15
  • 26
  • 2
    From the Readme: "This project will help identify potential security hotspots, but finds a lot of false positives which need triage by a human." I read that as "you're not supposed to fix them all". – Duncan Thacker Jul 10 '18 at 20:38
  • Disable this rule then? – Estus Flask Jul 10 '18 at 21:31
  • Sometimes it is required, when an outsource (like user input) could be involved. Check @viveksharma 's answer here: https://stackoverflow.com/a/55701580 – Tzahi Leh Sep 15 '20 at 13:19
  • 1
    This post explains why it can be a security issue: https://github.com/nodesecurity/eslint-plugin-security/blob/master/docs/the-dangers-of-square-bracket-notation.md In my opinion, you can `eslint-disable` it when you are (sure you are) not using user input for the `key` – publicJorn Dec 10 '20 at 12:33
  • can anyone help me with a question like this? [https://stackoverflow.com/questions/72295517/node-js-generic-object-injection-sink-on-eslint-using-for-iteration](https://stackoverflow.com/questions/72295517/node-js-generic-object-injection-sink-on-eslint-using-for-iteration) – r31sr4r May 18 '22 at 22:04

4 Answers4

23

You are searching for an ES lint error fix:

Here is the syntax for it

json [`${key}`]

Example:

const obj = { 
    eventName: 'Music event', 
    landingPic: 'landing.jpg',
    eventPic0: 'pic0.jpg',
    eventPic1: 'pic1.jpg',
    eventPic2: 'pic2.jpg',
    eventPic3: 'pic3.jpg',
    artist: 'Elie'
};

// array of keys which need to  be read
const arrayOfKey = ['landingPic', 'eventPic0', 'eventPic1',  'eventPic2',  'eventPic3'];

// let's read the value by a key in array
arrayOfKey.forEach( key => {
    const value = obj[`${key}`];
    console.log(value);
});
Divya Sakariya
  • 467
  • 4
  • 9
  • 3
    This may fix the ES Lint error, but this is a security issue. Does this help with that issue ??? – stefantigro Jul 28 '20 at 09:58
  • 1
    Apparently, line '12' (const arrayOfKey ...) is a whilelist of allowed keys. So yes, it helps with the security issue. – Robson William Feb 04 '21 at 15:52
  • Nice. Also this will convert the value of the key to a string. – Gabriel Anderson Sep 24 '21 at 11:14
  • 3
    You may as well just use `eslint-disable-next-line security/detect-object-injection` since you are writing code to make it safe. The language hack of converting it using back ticks (`\`${key}\``) is likely a bug in the eslint rule that should be fixed. – Ruan Mendes Mar 09 '22 at 15:04
  • This is clearly not a good way to fix this issue. The security issue is explained here https://github.com/eslint-community/eslint-plugin-security/blob/main/docs/the-dangers-of-square-bracket-notation.md So you should not use back ticks to avoid this rule, it is not making it safer. But in this example, there is no security issue. You can just disable it, if you concider it noisy – LETOURNEUR Léo Jan 30 '23 at 08:43
5

There is a good answer here. In general this rule is for paranoiac and the article to which everyone appeal is a mislead. So the best answer, I would say is to turn this rule off, if you can for sure.

And another answer in the comments refers to eslint contributor answer that this rule is pretty false positive prone and more for human to audit a codebase(warning level) rather then give an error in a CI. So I would say you can totally ignore this rule or turn it off.

If you cannot turn it off or ignore, you can disable the eslint for line with comment that it's a false positive or use some interpolation as mentioned in other answers.

And finally, in order to destroy any doubts, the answer from creator of the rule:

"I'm the original author of this rule - for a bit of context, it was originally written as an assistive tool for manual code reviews, to be used with the eslint plugin for VS Code. I would recommend disabling it for other use cases, as it's just going to be far too noisy."

Experimenter
  • 2,084
  • 1
  • 19
  • 26
2

Unsure why, but typecasting the access parameter silences the error. Guessing this has something to do with sanitation being able to prevent pollution.

const myThing = myObj[String(key)]
const myThing = myObj[key as string]
Geoff Seemueller
  • 21
  • 1
0

What its trying to say is that using this notation:

  • You are able to modify even prototype properties of the object which is considered dangerous
  • By being able to modify everything, you are also able to modify the constructor (method/function) so it may be injected and then exploited.

The subject is described analytically here, providing a simple example:

https://web.archive.org/web/20150430062816/https://blog.liftsecurity.io/2015/01/15/the-dangers-of-square-bracket-notation

Andreas
  • 416
  • 1
  • 4
  • 8