[PATCH] D73536: [analyser][taint] Remove taint from symbolic expressions if used in comparisons

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

In D73536#1845031 <https://reviews.llvm.org/D73536#1845031>, @NoQ wrote:

> > Describing value constraints in the taint config file is unfeasible.
>
> This is the only correct way to go, because, as you yourself point out, every sink function (or other use of tainted value) does indeed have different constraint requirements.


Over the last couple months I've been pretty conflicted on config files. While I see that it is the correct solution, I also fear that just like attributes, they require tedious work to set up and maintain. With that said, its been a while since I've evaluated analyses that had taint analysis in the focus, so I have no concrete data on whether its worth trying to reduce their count, though I suspect they wouldn't show the entire picture, as very few checkers utilize taintedness.

> What exactly is preventing you from describing value constraints in the config file?

This sounds like moving, or even worse duplicating the same checks both in a tool-specific config file and in the code. I sympathize with this as well:

>   int idx;
>   scanf("%d", &idx);
>   
>   if (idx < 0 || 42 < idx) { // tainted
>     return -1;
>   }
>   mySink(idx); // Warning {{Untrusted data is passed to a user-defined sink}}
>   return idx;
> 
> Even though we know at the point of `mySink` is called we know that `idx` is properly constrained, `mySink` will emit warning since `idx` holds tainted value.

This is valid, and I totally see how we can't possibly remove the taint (or in other words, prove to the analyzer that we properly checked the value) before passing it into a sink (as I understand it).

In summary, I think making decision like this is maybe a bit premature before we have some more results. It would be interesting to see what happens on larger projects once more checkers utilize taintedness, and act proactively, because

> Checking the wrong requirements is a very common source of security issues and we cannot afford destroying our ability to catch them.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73536/new/

https://reviews.llvm.org/D73536