[PATCH] D114706: [analyzer] Fix sensitive argument logic in GenericTaintChecker

Gabor Marton via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 7 08:49:11 PST 2022


martong added a comment.

> The semantics of taint sinks is that if ANY of the arguments is tainted, a
> warning should be emmitted. Before this change, if there were multiple
> arguments that are sensitive, and if the first arg is not tainted, but any of
> the noninitial are tainted, a warning is not emitted. After this change the
> correct semantics is reflected in code.

It worked well with custom sinks, didn't it? So, as I see, this patch is about system calls and subscript indices. Could you please update the description to have this information?



================
Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:951-957
+  // Find the first argument that receives a tainted value.
+  // The report is emitted as a side-effect.
+  return llvm::find_if(SensitiveArgs, [this, &C, &Call](unsigned Idx) {
+           return Idx < Call.getNumArgs() &&
+                  generateReportIfTainted(Call.getArgExpr(Idx), MsgCustomSink,
+                                          C);
+         }) != SensitiveArgs.end();
----------------
steakhal wrote:
> These algorithms are not supposed to have side-effects. Even though we technically could have side-effects, I would still apply it separately.
> That being said, this seems to be a copy-paste version of the previous hunk, like 3rd time.

> That being said, this seems to be a copy-paste version of the previous hunk, like 3rd time.
+1, to put it into a named function



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114706



More information about the cfe-commits mailing list