[JS] Taint does not propagate through .push(...) for arrays

[minusworld]

Describe the bug
It appears that taint mode stops propagating data at .push(...) in array objects.

To Reproduce
This link has 3 examples. The third does not work, but should.
https://semgrep.dev/s/bXKp

What is the priority of the bug to you?

• P0: blocking your adoption of Semgrep or workflow
• P1: important to fix or quite annoying
• P2: regular bug that should get fixed

[IagoAbal]

At present taint-mode only propagates taint through explicit assignments. In your example taint needs to be propagated through a side-effectful method call. We could perhaps asume that a (pressumably void-returning) method call like data.push(input) does taint data if input is tainted. We make similar assumptions in constant propagation already.

The current workaround would be:

pattern-sources:
  - patterns:
    - pattern-inside: |
        $ARRAY.push(<... req.query ...>)
        ...
    - pattern: $ARRAY

[IagoAbal]

Ooops, the workaround doesn't work in your example due to the limitations of ... in Semgrep. 😞

[minusworld]

Is it a good idea to allow someone to specify side-effectful regions which still propagate taint? That might be an easy way to permit this behavior on a case-by-case basis.

Another random idea I just had is to "chain" taint rules together, like source -> array.push(...), then array becomes a source for a new taint rule.

[aryx]

"We could perhaps asume that a (pressumably void-returning) method call like data.push(input) does taint data if input is tainted."
What if we did that instead of taint propagators? This is especially something we could do better in DeepSemgrep because we have more type information about the methods (but it's also easy to detect when something does not return anything because it's not used in an rvalue position.