Currently, we support constant propagation under limitations (globals in Python see #349; variables under the const keyword for other languages).
However, many of the checks we write involve a need to ensure that an expression evaluates to a compile-time constant. This is key to supporting the sort of secure-by-default work that, for instance, Google leverages heavily with things like detecting SQLi.
Desired behavior: expand constant propagation such that we don't need to rely on the const keyword. Pattern $X.innerHTML = "..." should match:
var term = random() > 0.5 ? 'baz' : 'baz2';
thing.innerHTML = term + ' — else';
Stretch/Non-goals:
($X: const<int>), even if the languages doesn't have a concept of const+1 this would be really helpful. Agreed that intraprocedural within 1 file would get most of the value (for now), at least based on how we write rules currently and community rule writing asks.
Per @ievans's first point, doing this for more than just strings would be nice (ints, floats, ...).
In practice, what users generally want to know is:
Is this
not a compile-time-constant; that is, could it be a dynamic value (and thus potentially user controlled)?
So when @ievans says $X.innerHTML = "...", this is actually the safe case, so your rule would probably combine that with a pattern-not like:
- patterns:
- pattern-not: $X.innerHTML = "..."; # filter out false positives
- pattern: $X.innerHTML = ...; # what remains is interesting
To save users some work combining clauses, would it be useful to provide a primitive like metavariable-regex but for checking that a metavariable is not a constant? Example:
- patterns:
- pattern: $X.innerHTML = $Y;
- metavariable-not-constant: "$Y"
Not sure how best to express this, but I've often wanted to be able to say, "... and X is not a compile time constant."
@IagoAbal is this something you would be interested in to do?
This sounds interesting, I'm taking it!
Here is another related example from a user:
https://semgrep.dev/s/3enD/
One thing we can fix, is to use eval_expr also in the VarDef case in Constant_propagation.ml, so if you do string yo = "foo" + "bar"; we resolve it.
But really what we need is a more general notion of "literalness", or "compile-time-constantness". People don't care about the value of the string literal; they use "..." in their pattern; they just want to make sure this is not a computed value (that could come from the user).
I think we can use the taint analysis infrastructure in lang_generic/analysis/Dataflow_tainting.ml to propagate this "compile-time-constantness" around. But first we need to fix AST_to_IL.ml and CFG_build.ml to implement all the missing cases.
I'm thinking that we could infer "constant-ness" in a flow-insensitive manner like we must be doing already for typing information.
We could have:
and const_info = Dyn | Cst | Lit of literal
and id_info = {
...
id_const_literal: const_info ref;
}
Here Cst would mean "some constant", something that we could infer in cases like var term = random() > 0.5 ? 'baz' : 'baz2';. Then a pattern like "..." could match an expression of type string with constant-ness Cst or Lit (String ...).
Eventually we could use dataflow analysis here, but that's a lot more powerful, then we could say "this variable has this value at this particular point in the program", regardless of whether it may have other values at other points.
Sounds good, yes we should define this new const_info type. We don't really compute typing information right now. We really just propagate types from their declaration in languages with type annotations (Java, Go) to their uses, and do very limited type inference on Go, but there's no typing analysis really right now.
Some of the features, such as($X: const<int>), are still not implemented.
Most helpful comment
+1 this would be really helpful. Agreed that intraprocedural within 1 file would get most of the value (for now), at least based on how we write rules currently and community rule writing asks.
Per @ievans's first point, doing this for more than just strings would be nice (ints, floats, ...).
In practice, what users generally want to know is:
So when @ievans says
$X.innerHTML = "...", this is actually the safe case, so your rule would probably combine that with apattern-notlike:To save users some work combining clauses, would it be useful to provide a primitive like metavariable-regex but for checking that a metavariable is not a constant? Example:
Not sure how best to express this, but I've often wanted to be able to say, "... and X is not a compile time constant."