[Javascript]: How to filter out StringLiteral source with LogicalOrExpr

[manunio]

Hi, I'm trying get my query to filter out const token = process.env.TOKEN || 'token value'; as source, Where const token variable's value has logical-or operator and right hand of that operator is a non-empty string literal.

For example:
js:

const token1 = process.env.TOKEN || 'token value';  // query should filter this out as source.

const token2 = 'token value'; // query should pick this up as source.

codeql query:

class NoneEmptyStringLiteral extends DataFlow::Node  {
  NoneEmptyStringLiteral() {
    this.asExpr().(StringLiteral).getValue().length() > 1 and
    not this.asExpr().(LogicalOrExpr).getRightOperand() instanceof StringLiteral
  }
}

Above query somehow works, but i'm not sure how the part not this.asExpr().(LogicalOrExpr).getRightOperand() instanceof StringLiteral is working.

What i'm confused about is what if i wish to get following line with comment.

const token1 = process.env.TOKEN || 'token value';   // query should pick this up as source.

const token2 = 'token value';

Removing negation does not seem to work.

class NoneEmptyStringLiteral extends DataFlow::Node  {
  NoneEmptyStringLiteral() {
    this.asExpr().(StringLiteral).getValue().length() > 1 and
    this.asExpr().(LogicalOrExpr).getRightOperand() instanceof StringLiteral // this does not have any effect.
  }
}

Hope this make sense :)

[erik-krogh]

You have a mistake in your NoneEmptyStringLiteral.
You write both this.asExpr().(StringLiteral) and this.asExpr().(LogicalOrExpr), which requires that the same dataflow-node is both a logical or and a stringliteral at the same time, which cannot happen.

I think you want something like this:

class LogicalOrWithNonEmptyString extends DataFlow::Node {
  LogicalOrWithNonEmptyString() {
    exists(LogicalOrExpr logOr | logOr = this.asExpr() |
      logOr.getAnOperand().(StringLiteral).getValue().lengt() > 1
    )
  }
}

Or maybe:

class LogicalOrWithNonEmptyString extends DataFlow::ValueNode {
  override LogicalOrExpr astNode;

  LogicalOrWithNonEmptyString() { astNode.getAnOperand().(StringLiteral).getValue() != "" }
}

[manunio]

Hi @erik-krogh thanks for the answer, it makes perfect sense, also can you please clear a doubt of mine.
say what if query needs to filter out following:

const token1 = process.env.TOKEN || 'token value';  // should avoid this
const token2 = 'token value'; // only pick this

as per what you said following is a mistake, so how can i only get line const token2 and filter out const token1.

class LogicalOrWithNonEmptyString extends DataFlow::Node {
  LogicalOrWithNonEmptyString() {
    this.asExpr() instanceof StringLiteral and // this is wrong too right ?
    not exists(LogicalOrExpr logOr | logOr = this.asExpr() |
      logOr.getAnOperand().(StringLiteral).getValue().length() > 1
    )
  }
}

[erik-krogh]

I think you want something like this:

  class LogicalOrWithNonEmptyString extends DataFlow::Node {
    LogicalOrWithNonEmptyString() {
      this.asExpr() instanceof StringLiteral and // <- technically redundant due to the next line
      this.asExpr().(StringLiteral).getValue().length() > 1 and
      not exists(LogicalOrExpr logOr | this.asExpr() = logOr.getAnOperand())
    }
  }

This will only select the first line (specifically the string constant in the first line).

[manunio]

I think you want something like this:

  class LogicalOrWithNonEmptyString extends DataFlow::Node {
    LogicalOrWithNonEmptyString() {
      this.asExpr() instanceof StringLiteral and // <- technically redundant due to the next line
      this.asExpr().(StringLiteral).getValue().length() > 1 and
      not exists(LogicalOrExpr logOr | this.asExpr() = logOr.getAnOperand())
    }
  }

This will only select the first line (specifically the string constant in the first line).

Thanks a lot, this makes perfect sense. i was over-complicating it. following query works for me :)

class LogicalOrWithNonEmptyString extends DataFlow::Node {
  LogicalOrWithNonEmptyString() {
    this.asExpr().(StringLiteral).getValue().length() > 1 and
    not exists(LogicalOrExpr logOr | this.asExpr() = logOr.getAnOperand())
  }
}