[clang] [analyzer] Adding taint analysis capability to unix.Malloc checker (PR #92420)
via cfe-commits
cfe-commits at lists.llvm.org
Thu May 23 01:18:13 PDT 2024
github-actions[bot] wrote:
<!--LLVM CODE FORMAT COMMENT: {clang-format}-->
:warning: C/C++ code formatter, clang-format found issues in your code. :warning:
<details>
<summary>
You can test this locally with the following command:
</summary>
``````````bash
git-clang-format --diff 1cde1240ed6e45012d7510f4aa39badbdb4a4721 b31ec694c88635404b252f00472140e83083fd02 -- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp clang/test/Analysis/malloc.c clang/test/Analysis/taint-diagnostic-visitor.c
``````````
</details>
<details>
<summary>
View the diff from clang-format here.
</summary>
``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 0167dd1336..84fc6700d2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1824,219 +1824,232 @@ void MallocChecker::reportTaintBug(StringRef Msg, ProgramStateRef State,
}
}
-void MallocChecker::CheckTaintedness(
- CheckerContext & C, const CallEvent &Call, const SVal SizeSVal,
- ProgramStateRef State, AllocationFamily Family) const {
- std::vector<SymbolRef> TaintedSyms =
- taint::getTaintedSymbols(State, SizeSVal);
- if (TaintedSyms.empty())
- return;
-
- SValBuilder &SVB = C.getSValBuilder();
- QualType SizeTy = SVB.getContext().getSizeType();
- QualType CmpTy = SVB.getConditionType();
- // In case the symbol is tainted, we give a warning if the
- // size is larger than SIZE_MAX/4
- BasicValueFactory &BVF = SVB.getBasicValueFactory();
- const llvm::APSInt MaxValInt = BVF.getMaxValue(SizeTy);
- NonLoc MaxLength =
- SVB.makeIntVal(MaxValInt / APSIntType(MaxValInt).getValue(4));
- std::optional<NonLoc> SizeNL = SizeSVal.getAs<NonLoc>();
- auto Cmp = SVB.evalBinOpNN(State, BO_GE, *SizeNL, MaxLength, CmpTy)
- .getAs<DefinedOrUnknownSVal>();
- if (!Cmp)
- return;
- auto [StateTooLarge, StateNotTooLarge] = State->assume(*Cmp);
- if (!StateTooLarge && StateNotTooLarge) {
- // we can prove that size is not too large so ok.
- return;
- }
+void MallocChecker::CheckTaintedness(CheckerContext &C, const CallEvent &Call,
+ const SVal SizeSVal, ProgramStateRef State,
+ AllocationFamily Family) const {
+ std::vector<SymbolRef> TaintedSyms =
+ taint::getTaintedSymbols(State, SizeSVal);
+ if (TaintedSyms.empty())
+ return;
- std::string Callee = "Memory allocation function";
- if (Call.getCalleeIdentifier())
- Callee = Call.getCalleeIdentifier()->getName().str();
- reportTaintBug(
- Callee + " is called with a tainted (potentially attacker controlled) "
- "value. Make sure the value is bound checked.",
- State, C, TaintedSyms, Family, Call.getArgExpr(0));
+ SValBuilder &SVB = C.getSValBuilder();
+ QualType SizeTy = SVB.getContext().getSizeType();
+ QualType CmpTy = SVB.getConditionType();
+ // In case the symbol is tainted, we give a warning if the
+ // size is larger than SIZE_MAX/4
+ BasicValueFactory &BVF = SVB.getBasicValueFactory();
+ const llvm::APSInt MaxValInt = BVF.getMaxValue(SizeTy);
+ NonLoc MaxLength =
+ SVB.makeIntVal(MaxValInt / APSIntType(MaxValInt).getValue(4));
+ std::optional<NonLoc> SizeNL = SizeSVal.getAs<NonLoc>();
+ auto Cmp = SVB.evalBinOpNN(State, BO_GE, *SizeNL, MaxLength, CmpTy)
+ .getAs<DefinedOrUnknownSVal>();
+ if (!Cmp)
+ return;
+ auto [StateTooLarge, StateNotTooLarge] = State->assume(*Cmp);
+ if (!StateTooLarge && StateNotTooLarge) {
+ // we can prove that size is not too large so ok.
+ return;
}
- ProgramStateRef MallocChecker::MallocMemAux(
- CheckerContext & C, const CallEvent &Call, SVal Size, SVal Init,
- ProgramStateRef State, AllocationFamily Family) const {
- if (!State)
- return nullptr;
+ std::string Callee = "Memory allocation function";
+ if (Call.getCalleeIdentifier())
+ Callee = Call.getCalleeIdentifier()->getName().str();
+ reportTaintBug(
+ Callee + " is called with a tainted (potentially attacker controlled) "
+ "value. Make sure the value is bound checked.",
+ State, C, TaintedSyms, Family, Call.getArgExpr(0));
+}
- const Expr *CE = Call.getOriginExpr();
+ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C,
+ const CallEvent &Call, SVal Size,
+ SVal Init, ProgramStateRef State,
+ AllocationFamily Family) const {
+ if (!State)
+ return nullptr;
- // We expect the malloc functions to return a pointer.
- if (!Loc::isLocType(CE->getType()))
- return nullptr;
+ const Expr *CE = Call.getOriginExpr();
- // Bind the return value to the symbolic value from the heap region.
- // TODO: move use of this functions to an EvalCall callback, becasue
- // BindExpr() should'nt be used elsewhere.
- unsigned Count = C.blockCount();
- SValBuilder &SVB = C.getSValBuilder();
- const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
- DefinedSVal RetVal =
- ((Family == AF_Alloca) ? SVB.getAllocaRegionVal(CE, LCtx, Count)
- : SVB.getConjuredHeapSymbolVal(CE, LCtx, Count)
- .castAs<DefinedSVal>());
- State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+ // We expect the malloc functions to return a pointer.
+ if (!Loc::isLocType(CE->getType()))
+ return nullptr;
- // Fill the region with the initialization value.
- State = State->bindDefaultInitial(RetVal, Init, LCtx);
+ // Bind the return value to the symbolic value from the heap region.
+ // TODO: move use of this functions to an EvalCall callback, becasue
+ // BindExpr() should'nt be used elsewhere.
+ unsigned Count = C.blockCount();
+ SValBuilder &SVB = C.getSValBuilder();
+ const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
+ DefinedSVal RetVal =
+ ((Family == AF_Alloca) ? SVB.getAllocaRegionVal(CE, LCtx, Count)
+ : SVB.getConjuredHeapSymbolVal(CE, LCtx, Count)
+ .castAs<DefinedSVal>());
+ State = State->BindExpr(CE, C.getLocationContext(), RetVal);
- // If Size is somehow undefined at this point, this line prevents a crash.
- if (Size.isUndef())
- Size = UnknownVal();
+ // Fill the region with the initialization value.
+ State = State->bindDefaultInitial(RetVal, Init, LCtx);
- CheckTaintedness(C, Call, Size, State, AF_Malloc);
+ // If Size is somehow undefined at this point, this line prevents a crash.
+ if (Size.isUndef())
+ Size = UnknownVal();
- // Set the region's extent.
- State = setDynamicExtent(State, RetVal.getAsRegion(),
- Size.castAs<DefinedOrUnknownSVal>(), SVB);
+ CheckTaintedness(C, Call, Size, State, AF_Malloc);
- return MallocUpdateRefState(C, CE, State, Family);
- }
+ // Set the region's extent.
+ State = setDynamicExtent(State, RetVal.getAsRegion(),
+ Size.castAs<DefinedOrUnknownSVal>(), SVB);
- static ProgramStateRef MallocUpdateRefState(
- CheckerContext & C, const Expr *E, ProgramStateRef State,
- AllocationFamily Family, std::optional<SVal> RetVal) {
- if (!State)
- return nullptr;
+ return MallocUpdateRefState(C, CE, State, Family);
+}
+
+static ProgramStateRef MallocUpdateRefState(CheckerContext &C, const Expr *E,
+ ProgramStateRef State,
+ AllocationFamily Family,
+ std::optional<SVal> RetVal) {
+ if (!State)
+ return nullptr;
- // Get the return value.
- if (!RetVal)
- RetVal = C.getSVal(E);
+ // Get the return value.
+ if (!RetVal)
+ RetVal = C.getSVal(E);
- // We expect the malloc functions to return a pointer.
- if (!RetVal->getAs<Loc>())
- return nullptr;
+ // We expect the malloc functions to return a pointer.
+ if (!RetVal->getAs<Loc>())
+ return nullptr;
- SymbolRef Sym = RetVal->getAsLocSymbol();
+ SymbolRef Sym = RetVal->getAsLocSymbol();
- // This is a return value of a function that was not inlined, such as
- // malloc() or new(). We've checked that in the caller. Therefore, it must
- // be a symbol.
- assert(Sym);
- // FIXME: In theory this assertion should fail for `alloca()` calls (because
- // `AllocaRegion`s are not symbolic); but in practice this does not happen.
- // As the current code appears to work correctly, I'm not touching this
- // issue now, but it would be good to investigate and clarify this. Also
- // note that perhaps the special `AllocaRegion` should be replaced by
- // `SymbolicRegion` (or turned into a subclass of `SymbolicRegion`) to
- // enable proper tracking of memory allocated by `alloca()` -- and after
- // that change this assertion would become valid again.
-
- // Set the symbol's state to Allocated.
- return State->set<RegionState>(Sym, RefState::getAllocated(Family, E));
- }
-
- ProgramStateRef MallocChecker::FreeMemAttr(
- CheckerContext & C, const CallEvent &Call, const OwnershipAttr *Att,
- ProgramStateRef State) const {
- if (!State)
- return nullptr;
+ // This is a return value of a function that was not inlined, such as
+ // malloc() or new(). We've checked that in the caller. Therefore, it must
+ // be a symbol.
+ assert(Sym);
+ // FIXME: In theory this assertion should fail for `alloca()` calls (because
+ // `AllocaRegion`s are not symbolic); but in practice this does not happen.
+ // As the current code appears to work correctly, I'm not touching this
+ // issue now, but it would be good to investigate and clarify this. Also
+ // note that perhaps the special `AllocaRegion` should be replaced by
+ // `SymbolicRegion` (or turned into a subclass of `SymbolicRegion`) to
+ // enable proper tracking of memory allocated by `alloca()` -- and after
+ // that change this assertion would become valid again.
+
+ // Set the symbol's state to Allocated.
+ return State->set<RegionState>(Sym, RefState::getAllocated(Family, E));
+}
+
+ProgramStateRef MallocChecker::FreeMemAttr(CheckerContext &C,
+ const CallEvent &Call,
+ const OwnershipAttr *Att,
+ ProgramStateRef State) const {
+ if (!State)
+ return nullptr;
- if (Att->getModule()->getName() != "malloc")
- return nullptr;
+ if (Att->getModule()->getName() != "malloc")
+ return nullptr;
- bool IsKnownToBeAllocated = false;
+ bool IsKnownToBeAllocated = false;
- for (const auto &Arg : Att->args()) {
- ProgramStateRef StateI =
- FreeMemAux(C, Call, State, Arg.getASTIndex(),
- Att->getOwnKind() == OwnershipAttr::Holds,
- IsKnownToBeAllocated, AF_Malloc);
- if (StateI)
- State = StateI;
- }
- return State;
+ for (const auto &Arg : Att->args()) {
+ ProgramStateRef StateI =
+ FreeMemAux(C, Call, State, Arg.getASTIndex(),
+ Att->getOwnKind() == OwnershipAttr::Holds,
+ IsKnownToBeAllocated, AF_Malloc);
+ if (StateI)
+ State = StateI;
}
+ return State;
+}
- ProgramStateRef MallocChecker::FreeMemAux(
- CheckerContext & C, const CallEvent &Call, ProgramStateRef State,
- unsigned Num, bool Hold, bool &IsKnownToBeAllocated,
- AllocationFamily Family, bool ReturnsNullOnFailure) const {
- if (!State)
- return nullptr;
+ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
+ const CallEvent &Call,
+ ProgramStateRef State, unsigned Num,
+ bool Hold, bool &IsKnownToBeAllocated,
+ AllocationFamily Family,
+ bool ReturnsNullOnFailure) const {
+ if (!State)
+ return nullptr;
- if (Call.getNumArgs() < (Num + 1))
- return nullptr;
+ if (Call.getNumArgs() < (Num + 1))
+ return nullptr;
- return FreeMemAux(C, Call.getArgExpr(Num), Call, State, Hold,
- IsKnownToBeAllocated, Family, ReturnsNullOnFailure);
- }
-
- /// Checks if the previous call to free on the given symbol failed - if free
- /// failed, returns true. Also, returns the corresponding return value symbol.
- static bool didPreviousFreeFail(ProgramStateRef State, SymbolRef Sym,
- SymbolRef & RetStatusSymbol) {
- const SymbolRef *Ret = State->get<FreeReturnValue>(Sym);
- if (Ret) {
- assert(*Ret && "We should not store the null return symbol");
- ConstraintManager &CMgr = State->getConstraintManager();
- ConditionTruthVal FreeFailed = CMgr.isNull(State, *Ret);
- RetStatusSymbol = *Ret;
- return FreeFailed.isConstrainedTrue();
- }
- return false;
- }
+ return FreeMemAux(C, Call.getArgExpr(Num), Call, State, Hold,
+ IsKnownToBeAllocated, Family, ReturnsNullOnFailure);
+}
- static bool printMemFnName(raw_ostream & os, CheckerContext & C,
- const Expr *E) {
- if (const CallExpr *CE = dyn_cast<CallExpr>(E)) {
- // FIXME: This doesn't handle indirect calls.
- const FunctionDecl *FD = CE->getDirectCallee();
- if (!FD)
- return false;
+/// Checks if the previous call to free on the given symbol failed - if free
+/// failed, returns true. Also, returns the corresponding return value symbol.
+static bool didPreviousFreeFail(ProgramStateRef State, SymbolRef Sym,
+ SymbolRef &RetStatusSymbol) {
+ const SymbolRef *Ret = State->get<FreeReturnValue>(Sym);
+ if (Ret) {
+ assert(*Ret && "We should not store the null return symbol");
+ ConstraintManager &CMgr = State->getConstraintManager();
+ ConditionTruthVal FreeFailed = CMgr.isNull(State, *Ret);
+ RetStatusSymbol = *Ret;
+ return FreeFailed.isConstrainedTrue();
+ }
+ return false;
+}
- os << *FD;
- if (!FD->isOverloadedOperator())
- os << "()";
- return true;
- }
+static bool printMemFnName(raw_ostream &os, CheckerContext &C, const Expr *E) {
+ if (const CallExpr *CE = dyn_cast<CallExpr>(E)) {
+ // FIXME: This doesn't handle indirect calls.
+ const FunctionDecl *FD = CE->getDirectCallee();
+ if (!FD)
+ return false;
- if (const ObjCMessageExpr *Msg = dyn_cast<ObjCMessageExpr>(E)) {
- if (Msg->isInstanceMessage())
- os << "-";
- else
- os << "+";
- Msg->getSelector().print(os);
- return true;
- }
+ os << *FD;
+ if (!FD->isOverloadedOperator())
+ os << "()";
+ return true;
+ }
- if (const CXXNewExpr *NE = dyn_cast<CXXNewExpr>(E)) {
- os << "'"
- << getOperatorSpelling(NE->getOperatorNew()->getOverloadedOperator())
- << "'";
- return true;
- }
+ if (const ObjCMessageExpr *Msg = dyn_cast<ObjCMessageExpr>(E)) {
+ if (Msg->isInstanceMessage())
+ os << "-";
+ else
+ os << "+";
+ Msg->getSelector().print(os);
+ return true;
+ }
- if (const CXXDeleteExpr *DE = dyn_cast<CXXDeleteExpr>(E)) {
- os << "'"
- << getOperatorSpelling(
- DE->getOperatorDelete()->getOverloadedOperator())
- << "'";
- return true;
- }
+ if (const CXXNewExpr *NE = dyn_cast<CXXNewExpr>(E)) {
+ os << "'"
+ << getOperatorSpelling(NE->getOperatorNew()->getOverloadedOperator())
+ << "'";
+ return true;
+ }
- return false;
+ if (const CXXDeleteExpr *DE = dyn_cast<CXXDeleteExpr>(E)) {
+ os << "'"
+ << getOperatorSpelling(DE->getOperatorDelete()->getOverloadedOperator())
+ << "'";
+ return true;
}
- static void printExpectedAllocName(raw_ostream & os,
- AllocationFamily Family) {
+ return false;
+}
- switch (Family) {
- case AF_Malloc: os << "malloc()"; return;
- case AF_CXXNew: os << "'new'"; return;
- case AF_CXXNewArray: os << "'new[]'"; return;
- case AF_IfNameIndex: os << "'if_nameindex()'"; return;
- case AF_InnerBuffer: os << "container-specific allocator"; return;
- case AF_Alloca:
- case AF_None: llvm_unreachable("not a deallocation expression");
+static void printExpectedAllocName(raw_ostream &os, AllocationFamily Family) {
+
+ switch (Family) {
+ case AF_Malloc:
+ os << "malloc()";
+ return;
+ case AF_CXXNew:
+ os << "'new'";
+ return;
+ case AF_CXXNewArray:
+ os << "'new[]'";
+ return;
+ case AF_IfNameIndex:
+ os << "'if_nameindex()'";
+ return;
+ case AF_InnerBuffer:
+ os << "container-specific allocator";
+ return;
+ case AF_Alloca:
+ case AF_None:
+ llvm_unreachable("not a deallocation expression");
}
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/92420
More information about the cfe-commits
mailing list