https://github.com/dkrupp created https://github.com/llvm/llvm-project/pull/112212
None >From 972c3089bffbce3516b711c4fc02df561b98433f Mon Sep 17 00:00:00 2001 From: Daniel Krupp <daniel.kr...@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.kr...@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.kr...@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.kr...@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.kr...@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.kr...@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.kr...@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.kr...@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++; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits