Python: Add StringConstCompare BarrierGuard

[RasmusWL]

I made a few other changes, since I was exploring how we handle BarrierGuards in general, and saw that I had not updated the logical test for sanitizers.

As mentioned in the commit message: In the future, I could imagine we would have something like this, but for now, I'm just keeping it simple.

module BarrierGuard {
  ...

  /**
   * A collection of common guards that ensure the checked value cannot have arbitrary
   * values.
   *
   * Currently only supports comparison with constant string value, but could also
   * include checking whether all characters are alphanumeric, or whether a regex is
   * matched against the value.
   *
   * Such guards will be useful for many taint-tracking queries, but not necessarily
   * all, which is why you need to opt into these manually.
   */
  class CommonNonArbitraryGuard extends BarrierGuard {
    CommonNonArbitraryGuard() {
      this instanceof StringConstCompare
    }

    override predicate checks(ControlFlowNode node, boolean branch) {
      this.(StringConstCompare).checks(node, branch)
    }
  }

[RasmusWL]

This is turning into a bit of a Frankensteins monster... sorry about that 👹 😞

[RasmusWL]

I think I tried to cut too many corners for StringConstCompare initially, by only supporting x == "safe", and not supporting x != "safe", x in ["safe"], or x not in ["safe"] -- so I added support for that.

My implementation doesn't try to handle variable references at all though 😞 We could do that, but I'm not sure it's the most important thing right now. Let me know if you think differently 😛

[tausbn]

A few questions/comments, otherwise this looks good to me. I like the test cases. 🙂

[tausbn]

This bit bugs me a bit, because it seems strangely inverted. Would it perhaps improve matters to define a separate abstract class of "constant iterables containing strings" and then extending it with subclasses for sequences and sets?

In particular, one thing that's unfortunate is that this doesn't account for local flow, which might be useful. One could extend the above to take local flow into consideration, but it would really blow up the size (in term of lines of code) of the characteristic predicate.

This also makes me think that perhaps these things should not live in DataFlowPublic (which is getting a bit crowded), but perhaps in a file dedicated solely to barrier guards?

[RasmusWL]

I agree. that code did not turn out that nice. I've tried to rewrite, let me know what you think :)

[tausbn]

Thanks for the changes. I think this looks good! 👍