[clang] Focused taint (PR #112215)

Daniel Krupp via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 14 07:29:32 PDT 2024


https://github.com/dkrupp created https://github.com/llvm/llvm-project/pull/112215

None

>From 972c3089bffbce3516b711c4fc02df561b98433f Mon Sep 17 00:00:00 2001
From: Daniel Krupp <daniel.krupp at ericsson.com>
Date: Mon, 3 Jun 2024 13:45:17 +0200
Subject: [PATCH 1/8] taint example code

---
 .../StaticAnalyzer/taint_focused/taintcalls.c | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 clang/lib/StaticAnalyzer/taint_focused/taintcalls.c

diff --git a/clang/lib/StaticAnalyzer/taint_focused/taintcalls.c b/clang/lib/StaticAnalyzer/taint_focused/taintcalls.c
new file mode 100644
index 00000000000000..d0e1e980d62be5
--- /dev/null
+++ b/clang/lib/StaticAnalyzer/taint_focused/taintcalls.c
@@ -0,0 +1,25 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+
+char buf[1024];
+
+int fetchTaintedString(char *txt){
+  scanf("%s", txt);
+}
+
+int exec(char* cmd){
+  system(cmd);//warn here
+}
+
+void topLevel(){
+  char cmd[2048] = "/bin/cat ";
+  char filename[1024];
+  fetchTaintedString (filename);
+  strcat(cmd, filename);
+  exec(cmd);
+}
+
+void printNum(int data){
+  printf("Data:%d\n",data);
+}
\ No newline at end of file

>From e5d5e07b76ec412586d07809e5237faee4204a91 Mon Sep 17 00:00:00 2001
From: Daniel Krupp <daniel.krupp at ericsson.com>
Date: Mon, 1 Jul 2024 05:47:57 +0200
Subject: [PATCH 2/8] [analyzer] Focused taint analysis

A new option is added to Clang Static Analyzer
that drives the symbolic execution to only
analyzer those top level functions which
may yield taint related findings.
---
 clang/include/clang/Driver/Options.td         |  3 +
 .../clang/StaticAnalyzer/Checkers/Checkers.td | 11 ++-
 .../StaticAnalyzer/Core/AnalyzerOptions.h     |  2 +
 .../Checkers/GenericTaintChecker.cpp          | 62 +++++++++++++++
 .../Frontend/AnalysisConsumer.cpp             | 78 +++++++++++++++++++
 5 files changed, 155 insertions(+), 1 deletion(-)

diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 3f4d1a328b4c27..9d5f2d3af62749 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -7005,6 +7005,9 @@ def analyzer_opt_analyze_headers : Flag<["-"], "analyzer-opt-analyze-headers">,
 def analyzer_display_progress : Flag<["-"], "analyzer-display-progress">,
   HelpText<"Emit verbose output about the analyzer's progress">,
   MarshallingInfoFlag<AnalyzerOpts<"AnalyzerDisplayProgress">>;
+def analyzer_focused_taint : Flag<["-"], "analyzer-focused-taint">,
+  HelpText<"Focus on taint analysis">,
+  MarshallingInfoFlag<AnalyzerOpts<"AnalyzerFocusedTaint">>;
 def analyzer_note_analysis_entry_points : Flag<["-"], "analyzer-note-analysis-entry-points">,
   HelpText<"Add a note for each bug report to denote their analysis entry points">,
   MarshallingInfoFlag<AnalyzerOpts<"AnalyzerNoteAnalysisEntryPoints">>;
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 7da0d0a87e8c0c..7e8d851ac7b818 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1048,7 +1048,6 @@ def ReturnPointerRangeChecker : Checker<"ReturnPtrRange">,
 
 } // end "alpha.security"
 
-
 //===----------------------------------------------------------------------===//
 // Mac OS X, Cocoa, and Core Foundation checkers.
 //===----------------------------------------------------------------------===//
@@ -1693,6 +1692,16 @@ def TaintPropagationChecker : Checker<"TaintPropagation">, // Modelling checker
 
 def GenericTaintChecker : Checker<"GenericTaint">,
   HelpText<"Reports potential injection vulnerabilities">,
+  CheckerOptions<[
+    CmdLineOption<Boolean,
+                  "AggressiveTaintPropagation",
+                  "If set to true and if a called function cannot be inlined, "
+                  "taintedness will be propagated to the return value and to non-const "
+                  "reference parameters",
+                  "false",
+                  Released,
+                  Hide>,
+  ]>,
   Dependencies<[TaintPropagationChecker]>,
   Documentation<HasDocumentation>;
 
diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
index 3a3c1a13d67dd5..1d21a03fdb960c 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -227,6 +227,7 @@ class AnalyzerOptions : public RefCountedBase<AnalyzerOptions> {
   unsigned ShouldEmitErrorsOnInvalidConfigValue : 1;
   unsigned AnalyzeAll : 1;
   unsigned AnalyzerDisplayProgress : 1;
+  unsigned AnalyzerFocusedTaint : 1;
   unsigned AnalyzerNoteAnalysisEntryPoints : 1;
 
   unsigned eagerlyAssumeBinOpBifurcation : 1;
@@ -293,6 +294,7 @@ class AnalyzerOptions : public RefCountedBase<AnalyzerOptions> {
         ShowConfigOptionsList(false),
         ShouldEmitErrorsOnInvalidConfigValue(false), AnalyzeAll(false),
         AnalyzerDisplayProgress(false), AnalyzerNoteAnalysisEntryPoints(false),
+        AnalyzerFocusedTaint(false),
         eagerlyAssumeBinOpBifurcation(false), TrimGraph(false),
         visualizeExplodedGraphWithGraphViz(false), UnoptimizedCFG(false),
         PrintStats(false), NoRetryExhausted(false), AnalyzerWerror(false) {}
diff --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index b89a6e2588c987..f8af9eedb689ca 100644
--- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -16,6 +16,7 @@
 
 #include "Yaml.h"
 #include "clang/AST/Attr.h"
+#include "clang/AST/Decl.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Checkers/Taint.h"
@@ -26,6 +27,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "llvm/ADT/ImmutableSet.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/YAMLTraits.h"
@@ -77,6 +79,13 @@ static ArgIdxTy fromArgumentCount(unsigned Count) {
   return Count;
 }
 
+/// Handles the resolution of indexes of type ArgIdxTy to Expr*-s.
+static const Expr *GetArgExpr(ArgIdxTy ArgIdx, const CallEvent &Call) {
+    return ArgIdx == ReturnValueIndex ? Call.getOriginExpr()
+                                      : Call.getArgExpr(ArgIdx);
+  };
+
+
 /// Check if the region the expression evaluates to is the standard input,
 /// and thus, is tainted.
 /// FIXME: Move this to Taint.cpp.
@@ -854,6 +863,8 @@ void GenericTaintChecker::checkPreCall(const CallEvent &Call,
 
 void GenericTaintChecker::checkPostCall(const CallEvent &Call,
                                         CheckerContext &C) const {
+
+
   // Set the marked values as tainted. The return value only accessible from
   // checkPostStmt.
   ProgramStateRef State = C.getState();
@@ -864,7 +875,58 @@ void GenericTaintChecker::checkPostCall(const CallEvent &Call,
   // stored in the state as TaintArgsOnPostVisit set.
   TaintArgsOnPostVisitTy TaintArgsMap = State->get<TaintArgsOnPostVisit>();
 
+
+  const ArgIdxTy CallNumArgs = fromArgumentCount(Call.getNumArgs());
+
+
   const ImmutableSet<ArgIdxTy> *TaintArgs = TaintArgsMap.lookup(CurrentFrame);
+  auto &F = State->getStateManager().get_context<ArgIdxFactory>();
+  ImmutableSet<ArgIdxTy> ApproxTaintedArgs = F.add(F.getEmptySet(), ReturnValueIndex);
+
+  if (!C.wasInlined && !TaintArgs) {
+    llvm::errs() << "PostCall<";
+    Call.dump(llvm::errs());
+    llvm::errs() << "Was not inlined.\n";
+    /// Check for taint sinks.
+    ProgramStateRef State = C.getState();
+    bool HasTaintedParam = false;
+    for (ArgIdxTy I = ReturnValueIndex; I < CallNumArgs; ++I) {
+      const Expr *E = GetArgExpr(I, Call);
+      if (!E)
+        continue;
+      HasTaintedParam =
+          HasTaintedParam || isTaintedOrPointsToTainted(State, C.getSVal(E));
+    }
+    llvm::errs() << "\nHasTaintedParam:" << HasTaintedParam << "\n";
+    bool AggressiveTaintPropagation =
+      C.getAnalysisManager().getAnalyzerOptions().getCheckerBooleanOption(
+          this, "AggressiveTaintPropagation");
+    if (HasTaintedParam && !TaintArgs && AggressiveTaintPropagation) {
+      llvm::errs() << "Making return value and writable params tainted.\n";
+      llvm::errs() << "Number of params:" << CallNumArgs;
+
+      for (ArgIdxTy I = ReturnValueIndex+1; I < CallNumArgs; ++I) {
+        const Expr *E = GetArgExpr(I, Call);
+        if (!E)
+          continue;
+        SVal ArgSVal = C.getSVal(E);
+        //checking if the argument is a non-const pointer
+        auto LValue = ArgSVal.getAs<Loc>();
+        if (!LValue)
+          continue;
+        const QualType ArgTy = LValue->getType(State->getStateManager().getContext());
+        if (ArgTy->isPointerType() && !ArgTy.isConstQualified()){
+          llvm::errs() << "Param "<< I;
+          llvm::errs() << "is a non const qualified pointer, so tainting it\n";
+          ArgTy->dump();
+          llvm::errs()<<"\n";
+          ApproxTaintedArgs = F.add(ApproxTaintedArgs, I);
+        }
+      }
+      TaintArgs = &ApproxTaintedArgs;
+    }
+  }
+
   if (!TaintArgs)
     return;
   assert(!TaintArgs->isEmpty());
diff --git a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
index 03bc40804d7328..c8f6caea64d2fc 100644
--- a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -33,6 +33,8 @@
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+#include <llvm/ADT/DepthFirstIterator.h>
+#include "clang/StaticAnalyzer/Core/PathSensitive/FunctionSummary.h"
 #include "llvm/ADT/PostOrderIterator.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/Statistic.h"
@@ -243,6 +245,11 @@ class AnalysisConsumer : public AnalysisASTConsumer,
   ExprEngine::InliningModes
     getInliningModeForFunction(const Decl *D, const SetOfConstDecls &Visited);
 
+
+  std::set<FunctionDecl*> getDeclsForTaintAnalysis(CallGraph &CG);
+  bool isTaintRelatedFunction(const FunctionDecl* FD);
+
+
   /// Build the call graph for all the top level decls of this TU and
   /// use it to define the order in which the functions should be visited.
   void HandleDeclsCallGraph(const unsigned LocalTUDeclsSize);
@@ -435,16 +442,75 @@ AnalysisConsumer::getInliningModeForFunction(const Decl *D,
   return ExprEngine::Inline_Regular;
 }
 
+bool AnalysisConsumer::isTaintRelatedFunction(const FunctionDecl* FD){
+  std::set<std::string> sources = {"scanf","gets","getch","read","fopen","fdopen","freopen","getchar",
+  "gets_s","scanf_s","getcwd","readlink","gethostname","getnameinfo","readlinkat","get_current_dir_name",
+  "getseuserbyname","getgroups","getlogin","getlogin_r","popen","getenv"};
+  std::set<std::string> sinks = {"system", "execv","popen","malloc","calloc","memcpy","strcpy","strncpy"};
+  if (!FD || !FD->getCanonicalDecl())
+    return false;
+  std::string FN = FD->getCanonicalDecl()->getNameAsString();
+  //llvm::errs()<<"Inspecting function if tainted " << FN << "\n";
+  return (sources.count(FN) || sinks.count(FN));
+}
+
+// returns functions declarations required for taint analysis
+std::set<FunctionDecl*> AnalysisConsumer::getDeclsForTaintAnalysis(CallGraph &CG) {
+  std::set<FunctionDecl*> TaintedFunctions;
+  for (auto N = llvm::df_begin(&CG), EI = llvm::df_end(&CG); N != EI; N++) {
+    Decl *D = N->getDecl();
+    // Skip the abstract root node.
+    if (!D)
+      continue;
+
+    auto *FD = dyn_cast<FunctionDecl>(D);
+    if (!FD)
+      continue;
+    if (FD->getDefinition()) {
+      llvm::errs() << "Visiting function: " << FD->getNameInfo().getAsString() << "\n";
+    }
+
+    for (auto Callee : N->callees()){
+      FunctionDecl *CFD = dyn_cast<FunctionDecl>(Callee.Callee->getDecl());
+      if (!CFD)
+        continue;
+      if (isTaintRelatedFunction(CFD))
+        TaintedFunctions.insert(FD);
+      if (TaintedFunctions.find(CFD)!=TaintedFunctions.end())//if child is tainted, the parent is also tainted
+        TaintedFunctions.insert(FD);
+    }
+
+
+    if (isTaintRelatedFunction(FD)){
+      llvm::errs()<<"Called Function "<<FD->getCanonicalDecl()->getNameAsString()<<" is tainted\n";
+      TaintedFunctions.insert(FD);
+    } else
+      llvm::errs()<<"Called Function "<<FD->getCanonicalDecl()->getNameAsString()<<" is NOT tainted\n";
+
+  }
+  return TaintedFunctions;
+}
+
 void AnalysisConsumer::HandleDeclsCallGraph(const unsigned LocalTUDeclsSize) {
   // Build the Call Graph by adding all the top level declarations to the graph.
   // Note: CallGraph can trigger deserialization of more items from a pch
   // (though HandleInterestingDecl); triggering additions to LocalTUDecls.
   // We rely on random access to add the initially processed Decls to CG.
+
   CallGraph CG;
   for (unsigned i = 0 ; i < LocalTUDeclsSize ; ++i) {
     CG.addToCallGraph(LocalTUDecls[i]);
   }
 
+  std::set<FunctionDecl*> TaintedFunctions;
+  if (Opts.AnalyzerFocusedTaint){
+    TaintedFunctions = getDeclsForTaintAnalysis(CG);
+    llvm::errs()<<"Tainted functions:\n";
+    for (FunctionDecl* FD:TaintedFunctions){
+      llvm::errs()<<FD->getNameInfo().getAsString() << "\n";
+    }
+  }
+
   // Walk over all of the call graph nodes in topological order, so that we
   // analyze parents before the children. Skip the functions inlined into
   // the previously processed functions. Use external Visited set to identify
@@ -463,6 +529,7 @@ void AnalysisConsumer::HandleDeclsCallGraph(const unsigned LocalTUDeclsSize) {
     if (!D)
       continue;
 
+
     // Skip the functions which have been processed already or previously
     // inlined.
     if (shouldSkipFunction(D, Visited, VisitedAsTopLevel))
@@ -480,6 +547,17 @@ void AnalysisConsumer::HandleDeclsCallGraph(const unsigned LocalTUDeclsSize) {
         continue;
     }
 
+    if (Opts.AnalyzerFocusedTaint) {
+      // if the function is not taint related skip it.
+      auto *FD = dyn_cast<FunctionDecl>(D);
+      if (TaintedFunctions.find(FD) == TaintedFunctions.end()) {
+        llvm::errs()
+            << "Skipping not taint related function from the analysis:\n";
+        llvm::errs() << FD->getNameInfo().getAsString() << "\n";
+        continue;
+      }
+    }
+
     // Analyze the function.
     SetOfConstDecls VisitedCallees;
 

>From 8ff5fcb12036ad685204e11141f6fbd63a55eee9 Mon Sep 17 00:00:00 2001
From: Daniel Krupp <daniel.krupp at ericsson.com>
Date: Thu, 3 Oct 2024 18:50:38 +0200
Subject: [PATCH 3/8] Add InlineTaintOnly feature

---
 clang/include/clang/Driver/Options.td           |  5 +++++
 .../clang/StaticAnalyzer/Core/AnalyzerOptions.h |  2 ++
 .../Core/PathSensitive/AnalysisManager.h        |  8 ++++++++
 .../Core/ExprEngineCallAndReturn.cpp            | 17 +++++++++++++++++
 .../Frontend/AnalysisConsumer.cpp               |  1 +
 5 files changed, 33 insertions(+)

diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 9d5f2d3af62749..55868e371a2f45 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -7008,6 +7008,11 @@ def analyzer_display_progress : Flag<["-"], "analyzer-display-progress">,
 def analyzer_focused_taint : Flag<["-"], "analyzer-focused-taint">,
   HelpText<"Focus on taint analysis">,
   MarshallingInfoFlag<AnalyzerOpts<"AnalyzerFocusedTaint">>;
+
+def analyzer_inline_taint_only : Flag<["-"], "analyzer-inline-only-tainted">,
+  HelpText<"Inline only tainted functions">,
+  MarshallingInfoFlag<AnalyzerOpts<"AnalyzerInlineTaintOnly">>;
+
 def analyzer_note_analysis_entry_points : Flag<["-"], "analyzer-note-analysis-entry-points">,
   HelpText<"Add a note for each bug report to denote their analysis entry points">,
   MarshallingInfoFlag<AnalyzerOpts<"AnalyzerNoteAnalysisEntryPoints">>;
diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
index 1d21a03fdb960c..23de8534d88197 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -228,6 +228,7 @@ class AnalyzerOptions : public RefCountedBase<AnalyzerOptions> {
   unsigned AnalyzeAll : 1;
   unsigned AnalyzerDisplayProgress : 1;
   unsigned AnalyzerFocusedTaint : 1;
+  unsigned AnalyzerInlineTaintOnly : 1;
   unsigned AnalyzerNoteAnalysisEntryPoints : 1;
 
   unsigned eagerlyAssumeBinOpBifurcation : 1;
@@ -295,6 +296,7 @@ class AnalyzerOptions : public RefCountedBase<AnalyzerOptions> {
         ShouldEmitErrorsOnInvalidConfigValue(false), AnalyzeAll(false),
         AnalyzerDisplayProgress(false), AnalyzerNoteAnalysisEntryPoints(false),
         AnalyzerFocusedTaint(false),
+        AnalyzerInlineTaintOnly(false),
         eagerlyAssumeBinOpBifurcation(false), TrimGraph(false),
         visualizeExplodedGraphWithGraphViz(false), UnoptimizedCFG(false),
         PrintStats(false), NoRetryExhausted(false), AnalyzerWerror(false) {}
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h
index c76e9c0326afe7..27d23714c1ea68 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h
@@ -42,6 +42,7 @@ class AnalysisManager : public BugReporterData {
   ConstraintManagerCreator CreateConstraintMgr;
 
   CheckerManager *CheckerMgr;
+  std::set<FunctionDecl*> TaintRelatedFunctions;
 
 public:
   AnalyzerOptions &options;
@@ -59,6 +60,13 @@ class AnalysisManager : public BugReporterData {
     AnaCtxMgr.clear();
   }
 
+  void setTaintRelatedFunctions(std::set<FunctionDecl*> TaintedFunctions){
+    TaintRelatedFunctions = TaintedFunctions;
+  }
+  std::set<FunctionDecl*> getTaintRelatedFunctions(){
+    return TaintRelatedFunctions;
+  }
+
   AnalysisDeclContextManager& getAnalysisDeclContextManager() {
     return AnaCtxMgr;
   }
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 2ca24d0c5ab22b..11f552c5bb4c79 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -1078,6 +1078,23 @@ bool ExprEngine::shouldInlineCall(const CallEvent &Call, const Decl *D,
   AnalysisDeclContextManager &ADCMgr = AMgr.getAnalysisDeclContextManager();
   AnalysisDeclContext *CalleeADC = ADCMgr.getContext(D);
 
+
+  if (Opts.AnalyzerInlineTaintOnly) {
+      std::set<FunctionDecl*> TaintedFunctions = AMgr.getTaintRelatedFunctions();
+      // if the function is not taint related skip it.
+      auto *FD = dyn_cast<FunctionDecl>(const_cast<Decl*>(D));
+      if (TaintedFunctions.find(FD) == TaintedFunctions.end()) {
+        llvm::errs()
+            << "Skipping inlining of not taint related function from the analysis:\n";
+        llvm::errs() << FD->getNameInfo().getAsString() << "\n";
+        return false;
+      } else {
+        llvm::errs()
+            << "inlining taint related function:\n";
+        llvm::errs() << FD->getNameInfo().getAsString() << "\n";
+        return true;
+      }
+  }
   // The auto-synthesized bodies are essential to inline as they are
   // usually small and commonly used. Note: we should do this check early on to
   // ensure we always inline these calls.
diff --git a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
index c8f6caea64d2fc..4980461dec2a5b 100644
--- a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -505,6 +505,7 @@ void AnalysisConsumer::HandleDeclsCallGraph(const unsigned LocalTUDeclsSize) {
   std::set<FunctionDecl*> TaintedFunctions;
   if (Opts.AnalyzerFocusedTaint){
     TaintedFunctions = getDeclsForTaintAnalysis(CG);
+    Mgr->setTaintRelatedFunctions(TaintedFunctions);
     llvm::errs()<<"Tainted functions:\n";
     for (FunctionDecl* FD:TaintedFunctions){
       llvm::errs()<<FD->getNameInfo().getAsString() << "\n";

>From 75b355283c510672e4dc709576ec43045fb4520b Mon Sep 17 00:00:00 2001
From: Daniel Krupp <daniel.krupp at ericsson.com>
Date: Thu, 3 Oct 2024 22:36:00 +0200
Subject: [PATCH 4/8] Moving taint related analyzer options under clangsa
 options

---
 clang/include/clang/Driver/Options.td                     | 8 --------
 .../include/clang/StaticAnalyzer/Core/AnalyzerOptions.def | 7 +++++++
 clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h | 2 --
 clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp    | 2 +-
 4 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 55868e371a2f45..3f4d1a328b4c27 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -7005,14 +7005,6 @@ def analyzer_opt_analyze_headers : Flag<["-"], "analyzer-opt-analyze-headers">,
 def analyzer_display_progress : Flag<["-"], "analyzer-display-progress">,
   HelpText<"Emit verbose output about the analyzer's progress">,
   MarshallingInfoFlag<AnalyzerOpts<"AnalyzerDisplayProgress">>;
-def analyzer_focused_taint : Flag<["-"], "analyzer-focused-taint">,
-  HelpText<"Focus on taint analysis">,
-  MarshallingInfoFlag<AnalyzerOpts<"AnalyzerFocusedTaint">>;
-
-def analyzer_inline_taint_only : Flag<["-"], "analyzer-inline-only-tainted">,
-  HelpText<"Inline only tainted functions">,
-  MarshallingInfoFlag<AnalyzerOpts<"AnalyzerInlineTaintOnly">>;
-
 def analyzer_note_analysis_entry_points : Flag<["-"], "analyzer-note-analysis-entry-points">,
   HelpText<"Add a note for each bug report to denote their analysis entry points">,
   MarshallingInfoFlag<AnalyzerOpts<"AnalyzerNoteAnalysisEntryPoints">>;
diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
index 737bc8e86cfb6a..c32947fc231a53 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
@@ -412,6 +412,13 @@ ANALYZER_OPTION(unsigned, MaxSymbolComplexity, "max-symbol-complexity",
 ANALYZER_OPTION(unsigned, MaxTaintedSymbolComplexity, "max-tainted-symbol-complexity",
                 "[DEPRECATED] The maximum complexity of a symbol to carry taint", 9)
 
+ANALYZER_OPTION(bool, AnalyzerInlineTaintOnly, "analyzer-inline-taint-only",
+                "Only inline on a call-path to a taint source or sink", false)
+
+ANALYZER_OPTION(bool, AnalyzerFocusedTaint, "analyzer-focused-taint",
+                "Only inline top-level functions on a call-path to a taint source or sink", false)
+
+
 ANALYZER_OPTION(unsigned, MaxTimesInlineLarge, "max-times-inline-large",
                 "The maximum times a large function could be inlined.", 32)
 
diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
index 23de8534d88197..75ca6089873223 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -227,8 +227,6 @@ class AnalyzerOptions : public RefCountedBase<AnalyzerOptions> {
   unsigned ShouldEmitErrorsOnInvalidConfigValue : 1;
   unsigned AnalyzeAll : 1;
   unsigned AnalyzerDisplayProgress : 1;
-  unsigned AnalyzerFocusedTaint : 1;
-  unsigned AnalyzerInlineTaintOnly : 1;
   unsigned AnalyzerNoteAnalysisEntryPoints : 1;
 
   unsigned eagerlyAssumeBinOpBifurcation : 1;
diff --git a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
index 4980461dec2a5b..2af0654e0a5cb6 100644
--- a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -503,7 +503,7 @@ void AnalysisConsumer::HandleDeclsCallGraph(const unsigned LocalTUDeclsSize) {
   }
 
   std::set<FunctionDecl*> TaintedFunctions;
-  if (Opts.AnalyzerFocusedTaint){
+  if (Opts.AnalyzerFocusedTaint||Opts.AnalyzerInlineTaintOnly){
     TaintedFunctions = getDeclsForTaintAnalysis(CG);
     Mgr->setTaintRelatedFunctions(TaintedFunctions);
     llvm::errs()<<"Tainted functions:\n";

>From 7a5e95a73857060896e02b5c4e7883b0df936196 Mon Sep 17 00:00:00 2001
From: Daniel Krupp <daniel.krupp at ericsson.com>
Date: Thu, 3 Oct 2024 22:49:19 +0200
Subject: [PATCH 5/8] do not inline a tainted function at all cost

---
 clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 11f552c5bb4c79..16e425537eb59f 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -1090,9 +1090,11 @@ bool ExprEngine::shouldInlineCall(const CallEvent &Call, const Decl *D,
         return false;
       } else {
         llvm::errs()
-            << "inlining taint related function:\n";
+            << "tyring to inline taint related function:\n";
         llvm::errs() << FD->getNameInfo().getAsString() << "\n";
-        return true;
+        //leave the other budget limits to kick in
+        //otherwise the analysis may hang
+        //return true;
       }
   }
   // The auto-synthesized bodies are essential to inline as they are

>From b706920029e519cbb516a2c009545a51c71e86d8 Mon Sep 17 00:00:00 2001
From: Daniel Krupp <daniel.krupp at ericsson.com>
Date: Sat, 5 Oct 2024 15:57:29 +0200
Subject: [PATCH 6/8] Move the AggressiveTaintPropagation option to the
 Tainpropagation checker from the GenericTaint checker

---
 .../clang/StaticAnalyzer/Checkers/Checkers.td  | 18 ++++++++----------
 .../Checkers/GenericTaintChecker.cpp           |  8 +++++---
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 7e8d851ac7b818..309d1bab74d7f5 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1685,23 +1685,21 @@ def TaintPropagationChecker : Checker<"TaintPropagation">, // Modelling checker
                   "Config",
                   "Specifies the name of the configuration file.",
                   "",
-                  Released>
-  ]>,
-  Documentation<NotDocumented>,
-  Hidden;
-
-def GenericTaintChecker : Checker<"GenericTaint">,
-  HelpText<"Reports potential injection vulnerabilities">,
-  CheckerOptions<[
+                  Released>,
     CmdLineOption<Boolean,
                   "AggressiveTaintPropagation",
                   "If set to true and if a called function cannot be inlined, "
                   "taintedness will be propagated to the return value and to non-const "
                   "reference parameters",
                   "false",
-                  Released,
-                  Hide>,
+                  Released>
+
   ]>,
+  Documentation<NotDocumented>,
+  Hidden;
+
+def GenericTaintChecker : Checker<"GenericTaint">,
+  HelpText<"Reports potential injection vulnerabilities">,
   Dependencies<[TaintPropagationChecker]>,
   Documentation<HasDocumentation>;
 
diff --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index f8af9eedb689ca..85b5796cc16701 100644
--- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -402,6 +402,7 @@ class GenericTaintChecker : public Checker<check::PreCall, check::PostCall> {
                                CheckerContext &C) const;
 
   bool isTaintReporterCheckerEnabled = false;
+  bool AggressiveTaintPropagation = false;
   std::optional<BugType> BT;
 
 private:
@@ -898,9 +899,6 @@ void GenericTaintChecker::checkPostCall(const CallEvent &Call,
           HasTaintedParam || isTaintedOrPointsToTainted(State, C.getSVal(E));
     }
     llvm::errs() << "\nHasTaintedParam:" << HasTaintedParam << "\n";
-    bool AggressiveTaintPropagation =
-      C.getAnalysisManager().getAnalyzerOptions().getCheckerBooleanOption(
-          this, "AggressiveTaintPropagation");
     if (HasTaintedParam && !TaintArgs && AggressiveTaintPropagation) {
       llvm::errs() << "Making return value and writable params tainted.\n";
       llvm::errs() << "Number of params:" << CallNumArgs;
@@ -1191,6 +1189,10 @@ void GenericTaintChecker::taintUnsafeSocketProtocol(const CallEvent &Call,
 /// Checker registration
 void ento::registerTaintPropagationChecker(CheckerManager &Mgr) {
   Mgr.registerChecker<GenericTaintChecker>();
+  GenericTaintChecker *checker = Mgr.getChecker<GenericTaintChecker>();
+  checker->AggressiveTaintPropagation =
+      Mgr.getAnalyzerOptions().getCheckerBooleanOption(checker,
+                                                  "AggressiveTaintPropagation");
 }
 
 bool ento::shouldRegisterTaintPropagationChecker(const CheckerManager &mgr) {

>From 1e3f3aea3e2efdfefb7269fe01c39ce1378a0533 Mon Sep 17 00:00:00 2001
From: Daniel Krupp <daniel.krupp at ericsson.com>
Date: Sun, 6 Oct 2024 20:45:15 +0200
Subject: [PATCH 7/8] Add AnalyzerAlwaysInlineTainted static analyzer parameter

---
 .../include/clang/StaticAnalyzer/Core/AnalyzerOptions.def | 3 +++
 clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h | 1 +
 clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp | 8 ++++++--
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
index c32947fc231a53..bab7491883396a 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
@@ -418,6 +418,9 @@ ANALYZER_OPTION(bool, AnalyzerInlineTaintOnly, "analyzer-inline-taint-only",
 ANALYZER_OPTION(bool, AnalyzerFocusedTaint, "analyzer-focused-taint",
                 "Only inline top-level functions on a call-path to a taint source or sink", false)
 
+ANALYZER_OPTION(bool, AnalyzerAlwaysInlineTainted, "analyzer-always-inline-tainted",
+                "Always inline taint related functions", false)
+
 
 ANALYZER_OPTION(unsigned, MaxTimesInlineLarge, "max-times-inline-large",
                 "The maximum times a large function could be inlined.", 32)
diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
index 75ca6089873223..0b979ecd94c9ea 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -295,6 +295,7 @@ class AnalyzerOptions : public RefCountedBase<AnalyzerOptions> {
         AnalyzerDisplayProgress(false), AnalyzerNoteAnalysisEntryPoints(false),
         AnalyzerFocusedTaint(false),
         AnalyzerInlineTaintOnly(false),
+        AnalyzerAlwaysInlineTainted(false),
         eagerlyAssumeBinOpBifurcation(false), TrimGraph(false),
         visualizeExplodedGraphWithGraphViz(false), UnoptimizedCFG(false),
         PrintStats(false), NoRetryExhausted(false), AnalyzerWerror(false) {}
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 16e425537eb59f..39d299b4ec9ea1 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -1078,6 +1078,7 @@ bool ExprEngine::shouldInlineCall(const CallEvent &Call, const Decl *D,
   AnalysisDeclContextManager &ADCMgr = AMgr.getAnalysisDeclContextManager();
   AnalysisDeclContext *CalleeADC = ADCMgr.getContext(D);
 
+  bool TaintRelatedFun=false;
 
   if (Opts.AnalyzerInlineTaintOnly) {
       std::set<FunctionDecl*> TaintedFunctions = AMgr.getTaintRelatedFunctions();
@@ -1089,6 +1090,7 @@ bool ExprEngine::shouldInlineCall(const CallEvent &Call, const Decl *D,
         llvm::errs() << FD->getNameInfo().getAsString() << "\n";
         return false;
       } else {
+        TaintRelatedFun=true;
         llvm::errs()
             << "tyring to inline taint related function:\n";
         llvm::errs() << FD->getNameInfo().getAsString() << "\n";
@@ -1137,15 +1139,17 @@ bool ExprEngine::shouldInlineCall(const CallEvent &Call, const Decl *D,
   }
 
   // Do not inline if recursive or we've reached max stack frame count.
+  // We skip this bound if the function is taint related
   bool IsRecursive = false;
   unsigned StackDepth = 0;
   examineStackFrames(D, Pred->getLocationContext(), IsRecursive, StackDepth);
-  if ((StackDepth >= Opts.InlineMaxStackDepth) &&
+  if (!(Opts.AnalyzerAlwaysInlineTainted && TaintRelatedFun) && (StackDepth >= Opts.InlineMaxStackDepth) &&
       (!isSmall(CalleeADC) || IsRecursive))
     return false;
 
   // Do not inline large functions too many times.
-  if ((Engine.FunctionSummaries->getNumTimesInlined(D) >
+  // We skip this bound if the function is taint related
+  if (!(Opts.AnalyzerAlwaysInlineTainted && TaintRelatedFun) && (Engine.FunctionSummaries->getNumTimesInlined(D) >
        Opts.MaxTimesInlineLarge) &&
       isLarge(CalleeADC)) {
     NumReachedInlineCountMax++;

>From 470cb4bbdeaa2f5238b28966a8557d7920f21bf1 Mon Sep 17 00:00:00 2001
From: Daniel Krupp <daniel.krupp at ericsson.com>
Date: Mon, 7 Oct 2024 23:06:23 +0200
Subject: [PATCH 8/8] limiting always inlne tainted option to 3x max stack
 depth

---
 clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 39d299b4ec9ea1..ce43548b2c66a2 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -1143,13 +1143,14 @@ bool ExprEngine::shouldInlineCall(const CallEvent &Call, const Decl *D,
   bool IsRecursive = false;
   unsigned StackDepth = 0;
   examineStackFrames(D, Pred->getLocationContext(), IsRecursive, StackDepth);
-  if (!(Opts.AnalyzerAlwaysInlineTainted && TaintRelatedFun) && (StackDepth >= Opts.InlineMaxStackDepth) &&
+  if (!(Opts.AnalyzerAlwaysInlineTainted && TaintRelatedFun && (StackDepth < 3*Opts.InlineMaxStackDepth))
+      && (StackDepth >= Opts.InlineMaxStackDepth) &&
       (!isSmall(CalleeADC) || IsRecursive))
     return false;
 
   // Do not inline large functions too many times.
   // We skip this bound if the function is taint related
-  if (!(Opts.AnalyzerAlwaysInlineTainted && TaintRelatedFun) && (Engine.FunctionSummaries->getNumTimesInlined(D) >
+  if ((Engine.FunctionSummaries->getNumTimesInlined(D) >
        Opts.MaxTimesInlineLarge) &&
       isLarge(CalleeADC)) {
     NumReachedInlineCountMax++;



More information about the cfe-commits mailing list