https://github.com/dkrupp updated https://github.com/llvm/llvm-project/pull/106389
>From ccc5da054903568fbd317d5c773251ed84f8f087 Mon Sep 17 00:00:00 2001 From: Daniel Krupp <daniel.kr...@ericsson.com> Date: Wed, 28 Aug 2024 15:32:35 +0200 Subject: [PATCH 1/2] Adding optin.taint.TaintedDiv checker Tainted division operation is separated out from the core.DivideZero checker into the optional optin.taint.TaintedDiv checker. The checker warns when the denominator in a division operation is an attacker controlled value. --- clang/docs/analyzer/checkers.rst | 28 +++++++++++ .../clang/StaticAnalyzer/Checkers/Checkers.td | 6 +++ .../StaticAnalyzer/Core/CheckerManager.h | 5 ++ .../Checkers/DivZeroChecker.cpp | 46 +++++++++++++++++-- .../test/Analysis/taint-diagnostic-visitor.c | 2 +- clang/test/Analysis/taint-generic.c | 3 ++ 6 files changed, 84 insertions(+), 6 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 89a1018e14c0e6..fab060302310bb 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -1053,6 +1053,34 @@ by explicitly marking the ``size`` parameter as sanitized. See the delete[] ptr; } +.. _optin-taint-TaintedDiv: + +optin.taint.TaintedDiv (C, C++, ObjC) +""""""""""""""""""""""""""""""""""""" +This checker warns when the denominator in a division +operation is a tainted (potentially attacker controlled) value. +If the attacker can set the denominator to 0, a runtime error can +be triggered. The checker warns if the analyzer cannot prove +that the denominator is not 0 and it is a tainted value. +This warning is more pessimistic than the :ref:`core-DivideZero` checker +which warns only when it can prove that the denominator is 0. + +.. code-block:: c + + int vulnerable(int n) { + size_t size = 0; + scanf("%zu", &size); + return n/size; // warn: Division by a tainted value, possibly zero + } + + int not_vulnerable(void) { + size_t size = 0; + scanf("%zu", &size); + if (!size) + return 0; + return n/size; // no warning + } + .. _security-checkers: security diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index fb4114619ac3d3..5a9afa0f15f5a0 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -1710,6 +1710,12 @@ def TaintedAllocChecker: Checker<"TaintedAlloc">, Dependencies<[DynamicMemoryModeling, TaintPropagationChecker]>, Documentation<HasDocumentation>; +def TaintedDivChecker: Checker<"TaintedDiv">, + HelpText<"Check for divisions, where the denominator " + "might be 0 as it is a tainted (attacker controlled) value.">, + Dependencies<[TaintPropagationChecker]>, + Documentation<HasDocumentation>; + } // end "optin.taint" //===----------------------------------------------------------------------===// diff --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h index ad25d18f280700..e47c0c310eb8e7 100644 --- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h +++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h @@ -223,6 +223,11 @@ class CheckerManager { return static_cast<CHECKER *>(CheckerTags[tag]); } + template <typename CHECKER> bool isRegisteredChecker() { + CheckerTag tag = getTag<CHECKER>(); + return (CheckerTags.count(tag) != 0); + } + //===----------------------------------------------------------------------===// // Functions for running checkers for AST traversing. //===----------------------------------------------------------------------===// diff --git a/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp index 5496f087447fbe..89aac24d7576e5 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp @@ -26,8 +26,6 @@ using namespace taint; namespace { class DivZeroChecker : public Checker< check::PreStmt<BinaryOperator> > { - const BugType BT{this, "Division by zero"}; - const BugType TaintBT{this, "Division by zero", categories::TaintedData}; void reportBug(StringRef Msg, ProgramStateRef StateZero, CheckerContext &C) const; void reportTaintBug(StringRef Msg, ProgramStateRef StateZero, @@ -35,6 +33,12 @@ class DivZeroChecker : public Checker< check::PreStmt<BinaryOperator> > { llvm::ArrayRef<SymbolRef> TaintedSyms) const; public: + /// This checker class implements multiple user facing checker + enum CheckKind { CK_DivZeroChecker, CK_TaintedDivChecker, CK_NumCheckKinds }; + bool ChecksEnabled[CK_NumCheckKinds] = {false}; + CheckerNameRef CheckNames[CK_NumCheckKinds]; + mutable std::unique_ptr<BugType> BugTypes[CK_NumCheckKinds]; + void checkPreStmt(const BinaryOperator *B, CheckerContext &C) const; }; } // end anonymous namespace @@ -48,8 +52,14 @@ static const Expr *getDenomExpr(const ExplodedNode *N) { void DivZeroChecker::reportBug(StringRef Msg, ProgramStateRef StateZero, CheckerContext &C) const { + if (!ChecksEnabled[CK_DivZeroChecker]) + return; + if (!BugTypes[CK_DivZeroChecker]) + BugTypes[CK_DivZeroChecker].reset( + new BugType(CheckNames[CK_DivZeroChecker], "Division by zero")); if (ExplodedNode *N = C.generateErrorNode(StateZero)) { - auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N); + auto R = std::make_unique<PathSensitiveBugReport>( + *BugTypes[CK_DivZeroChecker], Msg, N); bugreporter::trackExpressionValue(N, getDenomExpr(N), *R); C.emitReport(std::move(R)); } @@ -58,8 +68,15 @@ void DivZeroChecker::reportBug(StringRef Msg, ProgramStateRef StateZero, void DivZeroChecker::reportTaintBug( StringRef Msg, ProgramStateRef StateZero, CheckerContext &C, llvm::ArrayRef<SymbolRef> TaintedSyms) const { + if (!ChecksEnabled[CK_TaintedDivChecker]) + return; + if (!BugTypes[CK_TaintedDivChecker]) + BugTypes[CK_TaintedDivChecker].reset( + new BugType(CheckNames[CK_TaintedDivChecker], "Division by zero", + categories::TaintedData)); if (ExplodedNode *N = C.generateErrorNode(StateZero)) { - auto R = std::make_unique<PathSensitiveBugReport>(TaintBT, Msg, N); + auto R = std::make_unique<PathSensitiveBugReport>( + *BugTypes[CK_TaintedDivChecker], Msg, N); bugreporter::trackExpressionValue(N, getDenomExpr(N), *R); for (auto Sym : TaintedSyms) R->markInteresting(Sym); @@ -113,9 +130,28 @@ void DivZeroChecker::checkPreStmt(const BinaryOperator *B, } void ento::registerDivZeroChecker(CheckerManager &mgr) { - mgr.registerChecker<DivZeroChecker>(); + DivZeroChecker *checker = mgr.registerChecker<DivZeroChecker>(); + ; + checker->ChecksEnabled[DivZeroChecker::CK_DivZeroChecker] = true; + checker->CheckNames[DivZeroChecker::CK_DivZeroChecker] = + mgr.getCurrentCheckerName(); } bool ento::shouldRegisterDivZeroChecker(const CheckerManager &mgr) { return true; } + +void ento::registerTaintedDivChecker(CheckerManager &mgr) { + DivZeroChecker *checker; + if (!mgr.isRegisteredChecker<DivZeroChecker>()) + checker = mgr.registerChecker<DivZeroChecker>(); + else + checker = mgr.getChecker<DivZeroChecker>(); + checker->ChecksEnabled[DivZeroChecker::CK_TaintedDivChecker] = true; + checker->CheckNames[DivZeroChecker::CK_TaintedDivChecker] = + mgr.getCurrentCheckerName(); +} + +bool ento::shouldRegisterTaintedDivChecker(const CheckerManager &mgr) { + return true; +} diff --git a/clang/test/Analysis/taint-diagnostic-visitor.c b/clang/test/Analysis/taint-diagnostic-visitor.c index f51423646e8aec..11820003e8c4d8 100644 --- a/clang/test/Analysis/taint-diagnostic-visitor.c +++ b/clang/test/Analysis/taint-diagnostic-visitor.c @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2,optin.taint.TaintedAlloc -analyzer-output=text -verify %s +// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2,optin.taint.TaintedAlloc,optin.taint.TaintedDiv -analyzer-output=text -verify %s // This file is for testing enhanced diagnostics produced by the GenericTaintChecker diff --git a/clang/test/Analysis/taint-generic.c b/clang/test/Analysis/taint-generic.c index 1c139312734bca..e74e1a48ee40bb 100644 --- a/clang/test/Analysis/taint-generic.c +++ b/clang/test/Analysis/taint-generic.c @@ -1,6 +1,7 @@ // RUN: %clang_analyze_cc1 -Wno-format-security -Wno-pointer-to-int-cast \ // RUN: -Wno-incompatible-library-redeclaration -verify %s \ // RUN: -analyzer-checker=alpha.security.taint \ +// RUN: -analyzer-checker=optin.taint.TaintedDiv \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=alpha.security.ArrayBoundV2 \ // RUN: -analyzer-checker=debug.ExprInspection \ @@ -11,6 +12,7 @@ // RUN: -Wno-incompatible-library-redeclaration -verify %s \ // RUN: -DFILE_IS_STRUCT \ // RUN: -analyzer-checker=alpha.security.taint \ +// RUN: -analyzer-checker=optin.taint.TaintedDiv \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=alpha.security.ArrayBoundV2 \ // RUN: -analyzer-checker=debug.ExprInspection \ @@ -20,6 +22,7 @@ // RUN: not %clang_analyze_cc1 -Wno-pointer-to-int-cast \ // RUN: -Wno-incompatible-library-redeclaration -verify %s \ // RUN: -analyzer-checker=alpha.security.taint \ +// RUN: -analyzer-checker=optin.taint.TaintedDiv \ // RUN: -analyzer-checker=debug.ExprInspection \ // RUN: -analyzer-config \ // RUN: alpha.security.taint.TaintPropagation:Config=justguessit \ >From 5b52f2853358402c33c2f6b5e271ff6c4b383a15 Mon Sep 17 00:00:00 2001 From: Daniel Krupp <daniel.kr...@ericsson.com> Date: Wed, 28 Aug 2024 17:49:50 +0200 Subject: [PATCH 2/2] formatting fix --- clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp index 89aac24d7576e5..da160f13301413 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp @@ -25,7 +25,7 @@ using namespace ento; using namespace taint; namespace { -class DivZeroChecker : public Checker< check::PreStmt<BinaryOperator> > { +class DivZeroChecker : public Checker<check::PreStmt<BinaryOperator>> { void reportBug(StringRef Msg, ProgramStateRef StateZero, CheckerContext &C) const; void reportTaintBug(StringRef Msg, ProgramStateRef StateZero, _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits