From 0d339d06f8721d14befd6311bd306ac485772188 Mon Sep 17 00:00:00 2001 From: Anna Zaks <ganna@apple.com> Date: Thu, 17 Nov 2011 23:07:28 +0000 Subject: [PATCH] [analyzer] Do not conjure a symbol when we need to propagate taint. When the solver and SValBuilder cannot reason about symbolic expressions (ex: (x+1)*y ), the analyzer conjures a new symbol with no ties to the past. This helps it to recover some path-sensitivity. However, this breaks the taint propagation. With this commit, we are going to construct the expression even if we cannot reason about it later on if an operand is tainted. Also added some comments and asserts. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@144932 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Core/PathSensitive/SValBuilder.h | 18 +++++++++++-- lib/StaticAnalyzer/Core/ExprEngineC.cpp | 3 ++- lib/StaticAnalyzer/Core/SValBuilder.cpp | 25 +++++++++++++++++++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp | 14 +++++------ test/Analysis/taint-generic.c | 14 +++++++++++ 5 files changed, 64 insertions(+), 10 deletions(-) diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h b/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h index 17233e194e6..9266588163c 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h @@ -72,19 +72,33 @@ public: virtual SVal evalComplement(NonLoc val) = 0; + /// Create a new value which represents a binary expression with two non + /// location operands. virtual SVal evalBinOpNN(const ProgramState *state, BinaryOperator::Opcode op, NonLoc lhs, NonLoc rhs, QualType resultTy) = 0; + /// Create a new value which represents a binary expression with two memory + /// location operands. virtual SVal evalBinOpLL(const ProgramState *state, BinaryOperator::Opcode op, Loc lhs, Loc rhs, QualType resultTy) = 0; + /// Create a new value which represents a binary expression with a memory + /// location and non location operands. For example, this would be used to + /// evaluate a pointer arithmetic operation. virtual SVal evalBinOpLN(const ProgramState *state, BinaryOperator::Opcode op, Loc lhs, NonLoc rhs, QualType resultTy) = 0; - /// getKnownValue - evaluates a given SVal. If the SVal has only one possible - /// (integer) value, that value is returned. Otherwise, returns NULL. + /// Evaluates a given SVal. If the SVal has only one possible (integer) value, + /// that value is returned. Otherwise, returns NULL. virtual const llvm::APSInt *getKnownValue(const ProgramState *state, SVal val) = 0; + /// Handles generation of the value in case the builder is not smart enough to + /// handle the given binary expression. Depending on the state, decides to + /// either keep the expression or forget the history and generate an + /// UnknownVal. + SVal generateUnknownVal(const ProgramState *state, BinaryOperator::Opcode op, + NonLoc lhs, NonLoc rhs, QualType resultTy); + SVal evalBinOp(const ProgramState *state, BinaryOperator::Opcode op, SVal lhs, SVal rhs, QualType type); diff --git a/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/lib/StaticAnalyzer/Core/ExprEngineC.cpp index b82dfea58de..d74c48d962c 100644 --- a/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -361,7 +361,8 @@ void ExprEngine::VisitDeclStmt(const DeclStmt *DS, ExplodedNode *Pred, // UnknownVal. if ((InitVal.isUnknown() || !getConstraintManager().canReasonAbout(InitVal)) && - !VD->getType()->isReferenceType()) { + !VD->getType()->isReferenceType() && + !Pred->getState()->isTainted(InitVal)) { InitVal = svalBuilder.getConjuredSymbolVal(NULL, InitEx, currentBuilderContext->getCurrentBlockCount()); } diff --git a/lib/StaticAnalyzer/Core/SValBuilder.cpp b/lib/StaticAnalyzer/Core/SValBuilder.cpp index f118f4a0f09..db2097c16f2 100644 --- a/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ b/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -43,12 +43,14 @@ NonLoc SValBuilder::makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode op, // The Environment ensures we always get a persistent APSInt in // BasicValueFactory, so we don't need to get the APSInt from // BasicValueFactory again. + assert(lhs); assert(!Loc::isLocType(type)); return nonloc::SymExprVal(SymMgr.getSymIntExpr(lhs, op, rhs, type)); } NonLoc SValBuilder::makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode op, const SymExpr *rhs, QualType type) { + assert(lhs && rhs); assert(SymMgr.getType(lhs) == SymMgr.getType(rhs)); assert(!Loc::isLocType(type)); return nonloc::SymExprVal(SymMgr.getSymSymExpr(lhs, op, rhs, type)); @@ -162,6 +164,29 @@ DefinedSVal SValBuilder::getBlockPointer(const BlockDecl *block, //===----------------------------------------------------------------------===// +SVal SValBuilder::generateUnknownVal(const ProgramState *State, + BinaryOperator::Opcode Op, + NonLoc LHS, NonLoc RHS, + QualType ResultTy) { + // If operands are tainted, create a symbol to ensure that we propagate taint. + if (State->isTainted(RHS) || State->isTainted(LHS)) { + const SymExpr *symLHS; + const SymExpr *symRHS; + + if (const nonloc::ConcreteInt *rInt = dyn_cast<nonloc::ConcreteInt>(&RHS)) { + symLHS = LHS.getAsSymExpr(); + return makeNonLoc(symLHS, Op, rInt->getValue(), ResultTy); + } + // TODO: Handle the case when lhs is ConcreteInt. + + symLHS = LHS.getAsSymExpr(); + symRHS = RHS.getAsSymExpr(); + return makeNonLoc(symLHS, Op, symRHS, ResultTy); + } + return UnknownVal(); +} + + SVal SValBuilder::evalBinOp(const ProgramState *state, BinaryOperator::Opcode op, SVal lhs, SVal rhs, QualType type) { diff --git a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp index bd63ecf775d..f7924319e5b 100644 --- a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -298,7 +298,7 @@ SVal SimpleSValBuilder::evalBinOpNN(const ProgramState *state, while (1) { switch (lhs.getSubKind()) { default: - return UnknownVal(); + return generateUnknownVal(state, op, lhs, rhs, resultTy); case nonloc::LocAsIntegerKind: { Loc lhsL = cast<nonloc::LocAsInteger>(lhs).getLoc(); switch (rhs.getSubKind()) { @@ -321,7 +321,7 @@ SVal SimpleSValBuilder::evalBinOpNN(const ProgramState *state, return makeTruthVal(true, resultTy); default: // This case also handles pointer arithmetic. - return UnknownVal(); + return generateUnknownVal(state, op, lhs, rhs, resultTy); } } } @@ -333,7 +333,7 @@ SVal SimpleSValBuilder::evalBinOpNN(const ProgramState *state, dyn_cast<SymIntExpr>(selhs->getSymbolicExpression()); if (!symIntExpr) - return UnknownVal(); + return generateUnknownVal(state, op, lhs, rhs, resultTy); // Is this a logical not? (!x is represented as x == 0.) if (op == BO_EQ && rhs.isZeroConstant()) { @@ -381,7 +381,7 @@ SVal SimpleSValBuilder::evalBinOpNN(const ProgramState *state, // For now, only handle expressions whose RHS is a constant. const nonloc::ConcreteInt *rhsInt = dyn_cast<nonloc::ConcreteInt>(&rhs); if (!rhsInt) - return UnknownVal(); + return generateUnknownVal(state, op, lhs, rhs, resultTy); // If both the LHS and the current expression are additive, // fold their constants. @@ -467,9 +467,9 @@ SVal SimpleSValBuilder::evalBinOpNN(const ProgramState *state, if (lhsValue == 0) // At this point lhs and rhs have been swapped. return rhs; - return UnknownVal(); + return generateUnknownVal(state, op, lhs, rhs, resultTy); default: - return UnknownVal(); + return generateUnknownVal(state, op, lhs, rhs, resultTy); } } } @@ -529,7 +529,7 @@ SVal SimpleSValBuilder::evalBinOpNN(const ProgramState *state, resultTy); } - return UnknownVal(); + return generateUnknownVal(state, op, lhs, rhs, resultTy); } } } diff --git a/test/Analysis/taint-generic.c b/test/Analysis/taint-generic.c index 9179a57dad8..2e3def36709 100644 --- a/test/Analysis/taint-generic.c +++ b/test/Analysis/taint-generic.c @@ -12,3 +12,17 @@ void bufferFoo1(void) scanf("%d", &n); Buffer[n] = 1; // expected-warning {{Out of bound memory access }} } + +void bufferScanfArithmetic1(int x) { + int n; + scanf("%d", &n); + int m = (n - 3); + Buffer[m] = 1; // expected-warning {{Out of bound memory access }} +} + +void bufferScanfArithmetic2(int x) { + int n; + scanf("%d", &n); + int m = (n + 3) * x; + Buffer[m] = 1; // expected-warning {{Out of bound memory access }} +} -- GitLab