electronegativity icon indicating copy to clipboard operation
electronegativity copied to clipboard

Logical negation operator ! not considered in JS checks

Open ikkisoft opened this issue 4 years ago • 3 comments

Is your feature request related to a problem? Please describe. As reported in https://twitter.com/CryptoGangsta/status/1254223839497613312?s=20

We don't currently take into account configurations like:

  // Create the browser window.
  mainWindow = new BrowserWindow({
    width: 800,
    height: 600,
    webPreferences: {
      nodeIntegration: !0
    }
  })

We should check all boolean values for the ! sign and enhance the logic of several JS checks. Not sure if this is currently leading to false positives too.

ikkisoft avatar Apr 27 '20 07:04 ikkisoft

I added to the nodeIntegration check a first consideration for unary expressions such as !0 || !1. However this would still not cover more complicated cases such as !!0 || !!1 || !!!(2-1) and other combinations. I would rather cover the two basic cases (!0 || !1) for now and mark for manual review anything more complicated. If @ikkisoft is fine with this I would extend this approach to all the other checks.

An alternative solution would be to get the raw expression (e.g. using llafuente/esprima-ast-utils) and eval blindly whatever is in it, but I'm not enthusiastic about it.

phosphore avatar Sep 07 '20 11:09 phosphore

The proposed lazy solution seems reasonable to me.

ikkisoft avatar Oct 09 '20 10:10 ikkisoft

The proposed lazy solution seems reasonable to me.

The only way I can see us supporting such cases without using eval or the constructor function is by writing a basic BNF grammar to evaluate the most common expressions. But I'm sure we would still miss some edge cases. :shipit: I'll try to work on this

phosphore avatar Oct 09 '20 10:10 phosphore