Semgrep: Expand support for compile-time constants

Created on 4 Sep 2020  路  9Comments  路  Source: returntocorp/semgrep

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:

  • Although we usually see a need for compile-time-constant _strings_, it might be helpful to be able to specify const for any type in the program, e.g. ($X: const<int>), even if the languages doesn't have a concept of const
  • A basic implementation can be intraproceedural, with a stretch implementation being interproceedural but intrafile. (Related: equality binding constant propagation: https://github.com/returntocorp/semgrep/issues/374)
core enhancement

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:

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."

All 9 comments

+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.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ajinabraham picture ajinabraham  路  6Comments

brendongo picture brendongo  路  5Comments

ievans picture ievans  路  5Comments

msorens picture msorens  路  5Comments

dev-zero picture dev-zero  路  4Comments