diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index be455ae14f57a..114a963fce707 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -773,6 +773,11 @@ def DeprecatedOrUnsafeBufferHandling : Dependencies<[SecuritySyntaxChecker]>, Documentation; +def decodeValueOfObjCType : Checker<"decodeValueOfObjCType">, + HelpText<"Warn on uses of the '-decodeValueOfObjCType:at:' method">, + Dependencies<[SecuritySyntaxChecker]>, + Documentation; + } // end "security.insecureAPI" let ParentPackage = Security in { diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index f3a6befe0370c..f6e15d569a33f 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -2611,8 +2611,11 @@ static void RenderAnalyzerOptions(const ArgList &Args, ArgStringList &CmdArgs, CmdArgs.push_back("-analyzer-disable-checker=unix.Vfork"); } - if (Triple.isOSDarwin()) + if (Triple.isOSDarwin()) { CmdArgs.push_back("-analyzer-checker=osx"); + CmdArgs.push_back( + "-analyzer-checker=security.insecureAPI.decodeValueOfObjCType"); + } CmdArgs.push_back("-analyzer-checker=deadcode"); diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp index 260a2896e78c8..d9ffa562c0aaa 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp @@ -49,6 +49,7 @@ struct ChecksFilter { DefaultBool check_vfork; DefaultBool check_FloatLoopCounter; DefaultBool check_UncheckedReturn; + DefaultBool check_decodeValueOfObjCType; CheckerNameRef checkName_bcmp; CheckerNameRef checkName_bcopy; @@ -63,6 +64,7 @@ struct ChecksFilter { CheckerNameRef checkName_vfork; CheckerNameRef checkName_FloatLoopCounter; CheckerNameRef checkName_UncheckedReturn; + CheckerNameRef checkName_decodeValueOfObjCType; }; class WalkAST : public StmtVisitor { @@ -83,6 +85,7 @@ class WalkAST : public StmtVisitor { // Statement visitor methods. void VisitCallExpr(CallExpr *CE); + void VisitObjCMessageExpr(ObjCMessageExpr *CE); void VisitForStmt(ForStmt *S); void VisitCompoundStmt (CompoundStmt *S); void VisitStmt(Stmt *S) { VisitChildren(S); } @@ -93,6 +96,7 @@ class WalkAST : public StmtVisitor { bool checkCall_strCommon(const CallExpr *CE, const FunctionDecl *FD); typedef void (WalkAST::*FnCheck)(const CallExpr *, const FunctionDecl *); + typedef void (WalkAST::*MsgCheck)(const ObjCMessageExpr *); // Checker-specific methods. void checkLoopConditionForFloat(const ForStmt *FS); @@ -110,6 +114,7 @@ class WalkAST : public StmtVisitor { void checkCall_rand(const CallExpr *CE, const FunctionDecl *FD); void checkCall_random(const CallExpr *CE, const FunctionDecl *FD); void checkCall_vfork(const CallExpr *CE, const FunctionDecl *FD); + void checkMsg_decodeValueOfObjCType(const ObjCMessageExpr *ME); void checkUncheckedReturnValue(CallExpr *CE); }; } // end anonymous namespace @@ -182,6 +187,20 @@ void WalkAST::VisitCallExpr(CallExpr *CE) { VisitChildren(CE); } +void WalkAST::VisitObjCMessageExpr(ObjCMessageExpr *ME) { + MsgCheck evalFunction = + llvm::StringSwitch(ME->getSelector().getAsString()) + .Case("decodeValueOfObjCType:at:", + &WalkAST::checkMsg_decodeValueOfObjCType) + .Default(nullptr); + + if (evalFunction) + (this->*evalFunction)(ME); + + // Recurse and check children. + VisitChildren(ME); +} + void WalkAST::VisitCompoundStmt(CompoundStmt *S) { for (Stmt *Child : S->children()) if (Child) { @@ -923,6 +942,54 @@ void WalkAST::checkCall_vfork(const CallExpr *CE, const FunctionDecl *FD) { CELoc, CE->getCallee()->getSourceRange()); } +//===----------------------------------------------------------------------===// +// Check: '-decodeValueOfObjCType:at:' should not be used. +// It is deprecated in favor of '-decodeValueOfObjCType:at:size:' due to +// likelihood of buffer overflows. +//===----------------------------------------------------------------------===// + +void WalkAST::checkMsg_decodeValueOfObjCType(const ObjCMessageExpr *ME) { + if (!filter.check_decodeValueOfObjCType) + return; + + // Check availability of the secure alternative: + // iOS 11+, macOS 10.13+, tvOS 11+, and watchOS 4.0+ + // FIXME: We probably shouldn't register the check if it's not available. + const TargetInfo &TI = AC->getASTContext().getTargetInfo(); + const llvm::Triple &T = TI.getTriple(); + const VersionTuple &VT = TI.getPlatformMinVersion(); + switch (T.getOS()) { + case llvm::Triple::IOS: + if (VT < VersionTuple(11, 0)) + return; + break; + case llvm::Triple::MacOSX: + if (VT < VersionTuple(10, 13)) + return; + break; + case llvm::Triple::WatchOS: + if (VT < VersionTuple(4, 0)) + return; + break; + case llvm::Triple::TvOS: + if (VT < VersionTuple(11, 0)) + return; + break; + default: + return; + } + + PathDiagnosticLocation MELoc = + PathDiagnosticLocation::createBegin(ME, BR.getSourceManager(), AC); + BR.EmitBasicReport( + AC->getDecl(), filter.checkName_decodeValueOfObjCType, + "Potential buffer overflow in '-decodeValueOfObjCType:at:'", "Security", + "Deprecated method '-decodeValueOfObjCType:at:' is insecure " + "as it can lead to potential buffer overflows. Use the safer " + "'-decodeValueOfObjCType:at:size:' method.", + MELoc, ME->getSourceRange()); +} + //===----------------------------------------------------------------------===// // Check: Should check whether privileges are dropped successfully. // Originally: @@ -1035,3 +1102,4 @@ REGISTER_CHECKER(vfork) REGISTER_CHECKER(FloatLoopCounter) REGISTER_CHECKER(UncheckedReturn) REGISTER_CHECKER(DeprecatedOrUnsafeBufferHandling) +REGISTER_CHECKER(decodeValueOfObjCType) diff --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp index 978b0ce568b52..e64031dd67d0b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp @@ -24,9 +24,10 @@ #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" -#include "llvm/ADT/StringMap.h" #include "llvm/Support/YAMLTraits.h" +#include #include +#include #include using namespace clang; @@ -56,10 +57,11 @@ class GenericTaintChecker /// Used to parse the configuration file. struct TaintConfiguration { - using NameArgsPair = std::pair; + using NameScopeArgs = std::tuple; struct Propagation { std::string Name; + std::string Scope; ArgVector SrcArgs; SignedArgVector DstArgs; VariadicType VarType; @@ -67,8 +69,8 @@ class GenericTaintChecker }; std::vector Propagations; - std::vector Filters; - std::vector Sinks; + std::vector Filters; + std::vector Sinks; TaintConfiguration() = default; TaintConfiguration(const TaintConfiguration &) = default; @@ -97,18 +99,49 @@ class GenericTaintChecker BT.reset(new BugType(this, "Use of Untrusted Data", "Untrusted Data")); } + struct FunctionData { + FunctionData() = delete; + FunctionData(const FunctionData &) = default; + FunctionData(FunctionData &&) = default; + FunctionData &operator=(const FunctionData &) = delete; + FunctionData &operator=(FunctionData &&) = delete; + + static Optional create(const CallExpr *CE, + const CheckerContext &C) { + const FunctionDecl *FDecl = C.getCalleeDecl(CE); + if (!FDecl || (FDecl->getKind() != Decl::Function && + FDecl->getKind() != Decl::CXXMethod)) + return None; + + StringRef Name = C.getCalleeName(FDecl); + std::string FullName = FDecl->getQualifiedNameAsString(); + if (Name.empty() || FullName.empty()) + return None; + + return FunctionData{FDecl, Name, FullName}; + } + + bool isInScope(StringRef Scope) const { + return StringRef(FullName).startswith(Scope); + } + + const FunctionDecl *const FDecl; + const StringRef Name; + const std::string FullName; + }; + /// Catch taint related bugs. Check if tainted data is passed to a /// system call etc. Returns true on matching. - bool checkPre(const CallExpr *CE, const FunctionDecl *FDecl, StringRef Name, + bool checkPre(const CallExpr *CE, const FunctionData &FData, CheckerContext &C) const; /// Add taint sources on a pre-visit. Returns true on matching. - bool addSourcesPre(const CallExpr *CE, const FunctionDecl *FDecl, - StringRef Name, CheckerContext &C) const; + bool addSourcesPre(const CallExpr *CE, const FunctionData &FData, + CheckerContext &C) const; /// Mark filter's arguments not tainted on a pre-visit. Returns true on /// matching. - bool addFiltersPre(const CallExpr *CE, StringRef Name, + bool addFiltersPre(const CallExpr *CE, const FunctionData &FData, CheckerContext &C) const; /// Propagate taint generated at pre-visit. Returns true on matching. @@ -149,7 +182,7 @@ class GenericTaintChecker /// Check if tainted data is used as a custom sink's parameter. static constexpr llvm::StringLiteral MsgCustomSink = "Untrusted data is passed to a user-defined sink"; - bool checkCustomSinks(const CallExpr *CE, StringRef Name, + bool checkCustomSinks(const CallExpr *CE, const FunctionData &FData, CheckerContext &C) const; /// Generate a report if the expression is tainted or points to tainted data. @@ -157,8 +190,17 @@ class GenericTaintChecker CheckerContext &C) const; struct TaintPropagationRule; - using NameRuleMap = llvm::StringMap; - using NameArgMap = llvm::StringMap; + template + using ConfigDataMap = + std::unordered_multimap>; + using NameRuleMap = ConfigDataMap; + using NameArgMap = ConfigDataMap; + + /// Find a function with the given name and scope. Returns the first match + /// or the end of the map. + template + static auto findFunctionInConfig(const ConfigDataMap &Map, + const FunctionData &FData); /// A struct used to specify taint propagation rules for a function. /// @@ -200,8 +242,7 @@ class GenericTaintChecker /// Get the propagation rule for a given function. static TaintPropagationRule getTaintPropagationRule(const NameRuleMap &CustomPropagations, - const FunctionDecl *FDecl, StringRef Name, - CheckerContext &C); + const FunctionData &FData, CheckerContext &C); void addSrcArg(unsigned A) { SrcArgs.push_back(A); } void addDstArg(unsigned A) { DstArgs.push_back(A); } @@ -236,14 +277,15 @@ class GenericTaintChecker CheckerContext &C); }; - /// Defines a map between the propagation function's name and - /// TaintPropagationRule. + /// Defines a map between the propagation function's name, scope + /// and TaintPropagationRule. NameRuleMap CustomPropagations; - /// Defines a map between the filter function's name and filtering args. + /// Defines a map between the filter function's name, scope and filtering + /// args. NameArgMap CustomFilters; - /// Defines a map between the sink function's name and sinking args. + /// Defines a map between the sink function's name, scope and sinking args. NameArgMap CustomSinks; }; @@ -260,7 +302,7 @@ constexpr llvm::StringLiteral GenericTaintChecker::MsgCustomSink; using TaintConfig = GenericTaintChecker::TaintConfiguration; LLVM_YAML_IS_SEQUENCE_VECTOR(TaintConfig::Propagation) -LLVM_YAML_IS_SEQUENCE_VECTOR(TaintConfig::NameArgsPair) +LLVM_YAML_IS_SEQUENCE_VECTOR(TaintConfig::NameScopeArgs) namespace llvm { namespace yaml { @@ -275,6 +317,7 @@ template <> struct MappingTraits { template <> struct MappingTraits { static void mapping(IO &IO, TaintConfig::Propagation &Propagation) { IO.mapRequired("Name", Propagation.Name); + IO.mapOptional("Scope", Propagation.Scope); IO.mapOptional("SrcArgs", Propagation.SrcArgs); IO.mapOptional("DstArgs", Propagation.DstArgs); IO.mapOptional("VariadicType", Propagation.VarType, @@ -292,10 +335,11 @@ template <> struct ScalarEnumerationTraits { } }; -template <> struct MappingTraits { - static void mapping(IO &IO, TaintConfig::NameArgsPair &NameArg) { - IO.mapRequired("Name", NameArg.first); - IO.mapRequired("Args", NameArg.second); +template <> struct MappingTraits { + static void mapping(IO &IO, TaintConfig::NameScopeArgs &NSA) { + IO.mapRequired("Name", std::get<0>(NSA)); + IO.mapOptional("Scope", std::get<1>(NSA)); + IO.mapRequired("Args", std::get<2>(NSA)); } }; } // namespace yaml @@ -328,31 +372,51 @@ void GenericTaintChecker::parseConfiguration(CheckerManager &Mgr, const std::string &Option, TaintConfiguration &&Config) { for (auto &P : Config.Propagations) { - GenericTaintChecker::CustomPropagations.try_emplace( - P.Name, std::move(P.SrcArgs), - convertToArgVector(Mgr, Option, P.DstArgs), P.VarType, P.VarIndex); + GenericTaintChecker::CustomPropagations.emplace( + P.Name, + std::make_pair(P.Scope, TaintPropagationRule{ + std::move(P.SrcArgs), + convertToArgVector(Mgr, Option, P.DstArgs), + P.VarType, P.VarIndex})); } for (auto &F : Config.Filters) { - GenericTaintChecker::CustomFilters.try_emplace(F.first, - std::move(F.second)); + GenericTaintChecker::CustomFilters.emplace( + std::get<0>(F), + std::make_pair(std::move(std::get<1>(F)), std::move(std::get<2>(F)))); } for (auto &S : Config.Sinks) { - GenericTaintChecker::CustomSinks.try_emplace(S.first, std::move(S.second)); + GenericTaintChecker::CustomSinks.emplace( + std::get<0>(S), + std::make_pair(std::move(std::get<1>(S)), std::move(std::get<2>(S)))); } } +template +auto GenericTaintChecker::findFunctionInConfig(const ConfigDataMap &Map, + const FunctionData &FData) { + auto Range = Map.equal_range(FData.Name); + auto It = + std::find_if(Range.first, Range.second, [&FData](const auto &Entry) { + const auto &Value = Entry.second; + StringRef Scope = Value.first; + return Scope.empty() || FData.isInScope(Scope); + }); + return It != Range.second ? It : Map.end(); +} + GenericTaintChecker::TaintPropagationRule GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule( - const NameRuleMap &CustomPropagations, const FunctionDecl *FDecl, - StringRef Name, CheckerContext &C) { + const NameRuleMap &CustomPropagations, const FunctionData &FData, + CheckerContext &C) { // TODO: Currently, we might lose precision here: we always mark a return // value as tainted even if it's just a pointer, pointing to tainted data. // Check for exact name match for functions without builtin substitutes. + // Use qualified name, because these are C functions without namespace. TaintPropagationRule Rule = - llvm::StringSwitch(Name) + llvm::StringSwitch(FData.FullName) // Source functions // TODO: Add support for vfscanf & family. .Case("fdopen", TaintPropagationRule({}, {ReturnValueIndex})) @@ -397,6 +461,7 @@ GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule( // Check if it's one of the memory setting/copying functions. // This check is specialized but faster then calling isCLibraryFunction. + const FunctionDecl *FDecl = FData.FDecl; unsigned BId = 0; if ((BId = FDecl->getMemoryFunctionKind())) switch (BId) { @@ -440,35 +505,32 @@ GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule( // or smart memory copy: // - memccpy - copying until hitting a special character. - auto It = CustomPropagations.find(Name); - if (It != CustomPropagations.end()) - return It->getValue(); + auto It = findFunctionInConfig(CustomPropagations, FData); + if (It != CustomPropagations.end()) { + const auto &Value = It->second; + return Value.second; + } return TaintPropagationRule(); } void GenericTaintChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) const { - const FunctionDecl *FDecl = C.getCalleeDecl(CE); - // Check for non-global functions. - if (!FDecl || FDecl->getKind() != Decl::Function) - return; - - StringRef Name = C.getCalleeName(FDecl); - if (Name.empty()) + Optional FData = FunctionData::create(CE, C); + if (!FData) return; // Check for taintedness related errors first: system call, uncontrolled // format string, tainted buffer size. - if (checkPre(CE, FDecl, Name, C)) + if (checkPre(CE, *FData, C)) return; // Marks the function's arguments and/or return value tainted if it present in // the list. - if (addSourcesPre(CE, FDecl, Name, C)) + if (addSourcesPre(CE, *FData, C)) return; - addFiltersPre(CE, Name, C); + addFiltersPre(CE, *FData, C); } void GenericTaintChecker::checkPostStmt(const CallExpr *CE, @@ -484,12 +546,11 @@ void GenericTaintChecker::printState(raw_ostream &Out, ProgramStateRef State, } bool GenericTaintChecker::addSourcesPre(const CallExpr *CE, - const FunctionDecl *FDecl, - StringRef Name, + const FunctionData &FData, CheckerContext &C) const { // First, try generating a propagation rule for this function. TaintPropagationRule Rule = TaintPropagationRule::getTaintPropagationRule( - this->CustomPropagations, FDecl, Name, C); + this->CustomPropagations, FData, C); if (!Rule.isNull()) { ProgramStateRef State = Rule.process(CE, C); if (State) { @@ -500,14 +561,16 @@ bool GenericTaintChecker::addSourcesPre(const CallExpr *CE, return false; } -bool GenericTaintChecker::addFiltersPre(const CallExpr *CE, StringRef Name, +bool GenericTaintChecker::addFiltersPre(const CallExpr *CE, + const FunctionData &FData, CheckerContext &C) const { - auto It = CustomFilters.find(Name); + auto It = findFunctionInConfig(CustomFilters, FData); if (It == CustomFilters.end()) return false; ProgramStateRef State = C.getState(); - const ArgVector &Args = It->getValue(); + const auto &Value = It->second; + const ArgVector &Args = Value.second; for (unsigned ArgNum : Args) { if (ArgNum >= CE->getNumArgs()) continue; @@ -564,19 +627,19 @@ bool GenericTaintChecker::propagateFromPre(const CallExpr *CE, } bool GenericTaintChecker::checkPre(const CallExpr *CE, - const FunctionDecl *FDecl, StringRef Name, + const FunctionData &FData, CheckerContext &C) const { if (checkUncontrolledFormatString(CE, C)) return true; - if (checkSystemCall(CE, Name, C)) + if (checkSystemCall(CE, FData.Name, C)) return true; - if (checkTaintedBufferSize(CE, FDecl, C)) + if (checkTaintedBufferSize(CE, FData.FDecl, C)) return true; - if (checkCustomSinks(CE, Name, C)) + if (checkCustomSinks(CE, FData, C)) return true; return false; @@ -595,7 +658,7 @@ Optional GenericTaintChecker::getPointedToSVal(CheckerContext &C, QualType ArgTy = Arg->getType().getCanonicalType(); if (!ArgTy->isPointerType()) - return None; + return State->getSVal(*AddrLoc); QualType ValTy = ArgTy->getPointeeType(); @@ -848,13 +911,15 @@ bool GenericTaintChecker::checkTaintedBufferSize(const CallExpr *CE, generateReportIfTainted(CE->getArg(ArgNum), MsgTaintedBufferSize, C); } -bool GenericTaintChecker::checkCustomSinks(const CallExpr *CE, StringRef Name, +bool GenericTaintChecker::checkCustomSinks(const CallExpr *CE, + const FunctionData &FData, CheckerContext &C) const { - auto It = CustomSinks.find(Name); + auto It = findFunctionInConfig(CustomSinks, FData); if (It == CustomSinks.end()) return false; - const GenericTaintChecker::ArgVector &Args = It->getValue(); + const auto &Value = It->second; + const GenericTaintChecker::ArgVector &Args = Value.second; for (unsigned ArgNum : Args) { if (ArgNum >= CE->getNumArgs()) continue; diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 41d2c83829814..a8a9af20d6618 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -1469,6 +1469,9 @@ void MallocChecker::checkPostObjCMessage(const ObjCMethodCall &Call, if (!*FreeWhenDone) return; + if (Call.hasNonZeroCallbackArg()) + return; + bool IsKnownToBeAllocatedMemory; ProgramStateRef State = FreeMemAux(C, Call.getArgExpr(0), Call.getOriginExpr(), C.getState(), diff --git a/clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp index 43dbe57b8432b..6efba433eed22 100644 --- a/clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp @@ -36,6 +36,7 @@ class NonnullGlobalConstantsChecker : public Checker { mutable IdentifierInfo *NSStringII = nullptr; mutable IdentifierInfo *CFStringRefII = nullptr; mutable IdentifierInfo *CFBooleanRefII = nullptr; + mutable IdentifierInfo *CFNullRefII = nullptr; public: NonnullGlobalConstantsChecker() {} @@ -61,6 +62,7 @@ void NonnullGlobalConstantsChecker::initIdentifierInfo(ASTContext &Ctx) const { NSStringII = &Ctx.Idents.get("NSString"); CFStringRefII = &Ctx.Idents.get("CFStringRef"); CFBooleanRefII = &Ctx.Idents.get("CFBooleanRef"); + CFNullRefII = &Ctx.Idents.get("CFNullRef"); } /// Add an assumption that const string-like globals are non-null. @@ -136,7 +138,7 @@ bool NonnullGlobalConstantsChecker::isNonnullType(QualType Ty) const { T->getInterfaceDecl()->getIdentifier() == NSStringII; } else if (auto *T = dyn_cast(Ty)) { IdentifierInfo* II = T->getDecl()->getIdentifier(); - return II == CFStringRefII || II == CFBooleanRefII; + return II == CFStringRefII || II == CFBooleanRefII || II == CFNullRefII; } return false; } diff --git a/clang/lib/StaticAnalyzer/Core/Store.cpp b/clang/lib/StaticAnalyzer/Core/Store.cpp index b4ab6877726ce..b33129c88cea4 100644 --- a/clang/lib/StaticAnalyzer/Core/Store.cpp +++ b/clang/lib/StaticAnalyzer/Core/Store.cpp @@ -393,6 +393,11 @@ SVal StoreManager::attemptDownCast(SVal Base, QualType TargetType, return UnknownVal(); } +static bool hasSameUnqualifiedPointeeType(QualType ty1, QualType ty2) { + return ty1->getPointeeType().getCanonicalType().getTypePtr() == + ty2->getPointeeType().getCanonicalType().getTypePtr(); +} + /// CastRetrievedVal - Used by subclasses of StoreManager to implement /// implicit casts that arise from loads from regions that are reinterpreted /// as another region. @@ -421,10 +426,11 @@ SVal StoreManager::CastRetrievedVal(SVal V, const TypedValueRegion *R, // FIXME: We really need a single good function to perform casts for us // correctly every time we need it. if (castTy->isPointerType() && !castTy->isVoidPointerType()) - if (const auto *SR = dyn_cast_or_null(V.getAsRegion())) - if (SR->getSymbol()->getType().getCanonicalType() != - castTy.getCanonicalType()) - return loc::MemRegionVal(castRegion(SR, castTy)); + if (const auto *SR = dyn_cast_or_null(V.getAsRegion())) { + QualType sr = SR->getSymbol()->getType(); + if (!hasSameUnqualifiedPointeeType(sr, castTy)) + return loc::MemRegionVal(castRegion(SR, castTy)); + } return svalBuilder.dispatchCast(V, castTy); } diff --git a/clang/test/Analysis/Inputs/system-header-simulator-objc.h b/clang/test/Analysis/Inputs/system-header-simulator-objc.h index df751d03e642a..0dc6b369b0151 100644 --- a/clang/test/Analysis/Inputs/system-header-simulator-objc.h +++ b/clang/test/Analysis/Inputs/system-header-simulator-objc.h @@ -117,7 +117,10 @@ typedef double NSTimeInterval; + (id)dataWithBytesNoCopy:(void *)bytes length:(NSUInteger)length freeWhenDone:(BOOL)b; - (id)initWithBytesNoCopy:(void *)bytes length:(NSUInteger)length; - (id)initWithBytesNoCopy:(void *)bytes length:(NSUInteger)length freeWhenDone:(BOOL)b; -- (id)initWithBytes:(void *)bytes length:(NSUInteger) length; +- (id)initWithBytesNoCopy:(void *)bytes + length:(NSUInteger)length + deallocator:(void (^)(void *bytes, NSUInteger length))deallocator; +- (id)initWithBytes:(void *)bytes length:(NSUInteger)length; @end typedef struct { diff --git a/clang/test/Analysis/Inputs/taint-generic-config.yaml b/clang/test/Analysis/Inputs/taint-generic-config.yaml index 4605dd1c2efcc..39b52ccc32e67 100755 --- a/clang/test/Analysis/Inputs/taint-generic-config.yaml +++ b/clang/test/Analysis/Inputs/taint-generic-config.yaml @@ -9,12 +9,29 @@ Propagations: - Name: mySource2 DstArgs: [0] + # int x = myNamespace::mySource3(); // x is tainted + - Name: mySource3 + Scope: "myNamespace::" + DstArgs: [-1] + + # int x = myAnotherNamespace::mySource3(); // x is tainted + - Name: mySource3 + Scope: "myAnotherNamespace::" + DstArgs: [-1] + # int x, y; # myScanf("%d %d", &x, &y); // x and y are tainted - Name: myScanf VariadicType: Dst VariadicIndex: 1 + # int x, y; + # Foo::myScanf("%d %d", &x, &y); // x and y are tainted + - Name: myMemberScanf + Scope: "Foo::" + VariadicType: Dst + VariadicIndex: 1 + # int x; // x is tainted # int y; # myPropagator(x, &y); // y is tainted @@ -40,6 +57,18 @@ Filters: - Name: isOutOfRange Args: [0] + # int x; // x is tainted + # myNamespace::isOutOfRange(&x); // x is not tainted anymore + - Name: isOutOfRange2 + Scope: "myNamespace::" + Args: [0] + + # int x; // x is tainted + # myAnotherNamespace::isOutOfRange(&x); // x is not tainted anymore + - Name: isOutOfRange2 + Scope: "myAnotherNamespace::" + Args: [0] + # A list of sink functions Sinks: # int x, y; // x and y are tainted @@ -48,3 +77,15 @@ Sinks: # mySink(0, x, 1); // It won't warn - Name: mySink Args: [0, 2] + + # int x; // x is tainted + # myNamespace::mySink(x); // It will warn + - Name: mySink2 + Scope: "myNamespace::" + Args: [0] + + # int x; // x is tainted + # myAnotherNamespace::mySink(x); // It will warn + - Name: mySink2 + Scope: "myAnotherNamespace::" + Args: [0] diff --git a/clang/test/Analysis/malloc.mm b/clang/test/Analysis/malloc.mm index e84644b9dd732..1b7dd2756e1bb 100644 --- a/clang/test/Analysis/malloc.mm +++ b/clang/test/Analysis/malloc.mm @@ -1,4 +1,8 @@ -// RUN: %clang_analyze_cc1 -std=c++14 -analyzer-checker=core,unix.Malloc -analyzer-store=region -verify -fblocks %s +// RUN: %clang_analyze_cc1 -std=c++14 \ +// RUN: -analyzer-checker=core,unix.Malloc,cplusplus.NewDelete \ +// RUN: -analyzer-checker=unix.MismatchedDeallocator \ +// RUN: -verify -fblocks %s + #import "Inputs/system-header-simulator-objc.h" #import "Inputs/system-header-simulator-for-malloc.h" @@ -61,6 +65,23 @@ void testNSStringFreeWhenDoneNO(NSUInteger dataLength) { NSString *nsstr = [[NSString alloc] initWithBytesNoCopy:data length:dataLength encoding:NSUTF8StringEncoding freeWhenDone:0]; // expected-warning{{leak}} } +void testNSStringFreeWhenDoneNewDelete(NSUInteger dataLength) { + unsigned char *data = new unsigned char(42); + NSData *nsdata = [[NSData alloc] initWithBytesNoCopy:data + length:dataLength freeWhenDone:1]; + // expected-warning@-2{{-initWithBytesNoCopy:length:freeWhenDone: cannot take ownership of memory allocated by 'new'}} +} + +void testNSStringFreeWhenDoneNewDelete2(NSUInteger dataLength) { + unsigned char *data = new unsigned char(42); + NSData *nsdata = [[NSData alloc] initWithBytesNoCopy:data + length:dataLength + deallocator:^(void *bytes, + NSUInteger length) { + delete (unsigned char *)bytes; + }]; // no-warning +} + void testNSStringFreeWhenDoneNO2(NSUInteger dataLength) { unichar *data = (unichar*)malloc(42); NSString *nsstr = [[NSString alloc] initWithCharactersNoCopy:data length:dataLength freeWhenDone:0]; // expected-warning{{leak}} diff --git a/clang/test/Analysis/nonnull-global-constants.mm b/clang/test/Analysis/nonnull-global-constants.mm index 9e1a588ba47a8..8c174c48b30d6 100644 --- a/clang/test/Analysis/nonnull-global-constants.mm +++ b/clang/test/Analysis/nonnull-global-constants.mm @@ -7,7 +7,11 @@ @class NSString; typedef const struct __CFString *CFStringRef; -typedef const struct __CFBoolean * CFBooleanRef; +typedef const struct __CFBoolean *CFBooleanRef; + +#define CF_BRIDGED_TYPE(T) __attribute__((objc_bridge(T))) +typedef const struct CF_BRIDGED_TYPE(NSNull) __CFNull *CFNullRef; +extern const CFNullRef kCFNull; // Global NSString* is non-null. extern NSString *const StringConstGlobal; @@ -113,3 +117,7 @@ void testNonnullNonconstCFString() { void testNonnullNonnullCFString() { clang_analyzer_eval(str4); // expected-warning{{TRUE}} } + +void test_kCFNull() { + clang_analyzer_eval(kCFNull); // expected-warning{{TRUE}} +} diff --git a/clang/test/Analysis/security-syntax-checks-nscoder.m b/clang/test/Analysis/security-syntax-checks-nscoder.m new file mode 100644 index 0000000000000..23aa95bedccdc --- /dev/null +++ b/clang/test/Analysis/security-syntax-checks-nscoder.m @@ -0,0 +1,36 @@ +// RUN: %clang_analyze_cc1 -triple thumbv7-apple-ios11.0 -verify=available \ +// RUN: -analyzer-checker=security.insecureAPI.decodeValueOfObjCType %s +// RUN: %clang_analyze_cc1 -triple thumbv7-apple-ios10.0 -verify=notavailable \ +// RUN: -analyzer-checker=security.insecureAPI.decodeValueOfObjCType %s +// RUN: %clang_analyze_cc1 -triple x86_64-apple-macos10.13 -verify=available \ +// RUN: -analyzer-checker=security.insecureAPI.decodeValueOfObjCType %s +// RUN: %clang_analyze_cc1 -triple x86_64-apple-macos10.12 -verify=notavailable \ +// RUN: -analyzer-checker=security.insecureAPI.decodeValueOfObjCType %s +// RUN: %clang_analyze_cc1 -triple thumbv7-apple-watchos4.0 -verify=available \ +// RUN: -analyzer-checker=security.insecureAPI.decodeValueOfObjCType %s +// RUN: %clang_analyze_cc1 -triple thumbv7-apple-watchos3.0 -verify=notavailable \ +// RUN: -analyzer-checker=security.insecureAPI.decodeValueOfObjCType %s +// RUN: %clang_analyze_cc1 -triple thumbv7-apple-tvos11.0 -verify=available \ +// RUN: -analyzer-checker=security.insecureAPI.decodeValueOfObjCType %s +// RUN: %clang_analyze_cc1 -triple thumbv7-apple-tvos10.0 -verify=notavailable \ +// RUN: -analyzer-checker=security.insecureAPI.decodeValueOfObjCType %s + +// notavailable-no-diagnostics + +typedef unsigned long NSUInteger; + +@interface NSCoder +- (void)decodeValueOfObjCType:(const char *)type + at:(void *)data; +- (void)decodeValueOfObjCType:(const char *)type + at:(void *)data + size:(NSUInteger)size; +@end + +void test(NSCoder *decoder) { + // This would be a vulnerability on 64-bit platforms + // but not on 32-bit platforms. + NSUInteger x; + [decoder decodeValueOfObjCType:"I" at:&x]; // available-warning{{Deprecated method '-decodeValueOfObjCType:at:' is insecure as it can lead to potential buffer overflows. Use the safer '-decodeValueOfObjCType:at:size:' method}} + [decoder decodeValueOfObjCType:"I" at:&x size:sizeof(x)]; // no-warning +} diff --git a/clang/test/Analysis/taint-generic.cpp b/clang/test/Analysis/taint-generic.cpp new file mode 100644 index 0000000000000..09cd54471948e --- /dev/null +++ b/clang/test/Analysis/taint-generic.cpp @@ -0,0 +1,126 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -analyzer-config alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml -Wno-format-security -verify -std=c++11 %s + +#define BUFSIZE 10 +int Buffer[BUFSIZE]; + +int scanf(const char*, ...); +int mySource1(); +int mySource3(); + +bool isOutOfRange2(const int*); + +void mySink2(int); + +// Test configuration +namespace myNamespace { + void scanf(const char*, ...); + void myScanf(const char*, ...); + int mySource3(); + + bool isOutOfRange(const int*); + bool isOutOfRange2(const int*); + + void mySink(int, int, int); + void mySink2(int); +} + +namespace myAnotherNamespace { + int mySource3(); + + bool isOutOfRange2(const int*); + + void mySink2(int); +} + +void testConfigurationNamespacePropagation1() { + int x; + // The built-in functions should be matched only for functions in + // the global namespace + myNamespace::scanf("%d", &x); + Buffer[x] = 1; // no-warning + + scanf("%d", &x); + Buffer[x] = 1; // expected-warning {{Out of bound memory access }} +} + +void testConfigurationNamespacePropagation2() { + int x = mySource3(); + Buffer[x] = 1; // no-warning + + int y = myNamespace::mySource3(); + Buffer[y] = 1; // expected-warning {{Out of bound memory access }} +} + +void testConfigurationNamespacePropagation3() { + int x = myAnotherNamespace::mySource3(); + Buffer[x] = 1; // expected-warning {{Out of bound memory access }} +} + +void testConfigurationNamespacePropagation4() { + int x; + // Configured functions without scope should match for all function. + myNamespace::myScanf("%d", &x); + Buffer[x] = 1; // expected-warning {{Out of bound memory access }} +} + +void testConfigurationNamespaceFilter1() { + int x = mySource1(); + if (myNamespace::isOutOfRange2(&x)) + return; + Buffer[x] = 1; // no-warning + + int y = mySource1(); + if (isOutOfRange2(&y)) + return; + Buffer[y] = 1; // expected-warning {{Out of bound memory access }} +} + +void testConfigurationNamespaceFilter2() { + int x = mySource1(); + if (myAnotherNamespace::isOutOfRange2(&x)) + return; + Buffer[x] = 1; // no-warning +} + +void testConfigurationNamespaceFilter3() { + int x = mySource1(); + if (myNamespace::isOutOfRange(&x)) + return; + Buffer[x] = 1; // no-warning +} + +void testConfigurationNamespaceSink1() { + int x = mySource1(); + mySink2(x); // no-warning + + int y = mySource1(); + myNamespace::mySink2(y); + // expected-warning@-1 {{Untrusted data is passed to a user-defined sink}} +} + +void testConfigurationNamespaceSink2() { + int x = mySource1(); + myAnotherNamespace::mySink2(x); + // expected-warning@-1 {{Untrusted data is passed to a user-defined sink}} +} + +void testConfigurationNamespaceSink3() { + int x = mySource1(); + myNamespace::mySink(x, 0, 1); + // expected-warning@-1 {{Untrusted data is passed to a user-defined sink}} +} + +struct Foo { + void scanf(const char*, int*); + void myMemberScanf(const char*, int*); +}; + +void testConfigurationMemberFunc() { + int x; + Foo foo; + foo.scanf("%d", &x); + Buffer[x] = 1; // no-warning + + foo.myMemberScanf("%d", &x); + Buffer[x] = 1; // expected-warning {{Out of bound memory access }} +} diff --git a/clang/test/Analysis/uninit-val-const-likeness.c b/clang/test/Analysis/uninit-val-const-likeness.c new file mode 100644 index 0000000000000..013ab7882755a --- /dev/null +++ b/clang/test/Analysis/uninit-val-const-likeness.c @@ -0,0 +1,74 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core %s -verify +// expected-no-diagnostics + +#define SIZE 2 + +typedef struct { + int noOfSymbols; +} Params; + +static void create(const Params * const params, int fooList[]) { + int tmpList[SIZE] = {0}; + for (int i = 0; i < params->noOfSymbols; i++) + fooList[i] = tmpList[i]; +} + +int work(Params * const params) { + int fooList[SIZE]; + create(params, fooList); + int sum = 0; + for (int i = 0; i < params->noOfSymbols; i++) + sum += fooList[i]; // no-warning + return sum; +} + +static void create2(const Params * const * pparams, int fooList[]) { + const Params * params = *pparams; + int tmpList[SIZE] = {0}; + for (int i = 0; i < params->noOfSymbols; i++) + fooList[i] = tmpList[i]; +} + +int work2(const Params * const params) { + int fooList[SIZE]; + create2(¶ms, fooList); + int sum = 0; + for (int i = 0; i < params->noOfSymbols; i++) + sum += fooList[i]; // no-warning + return sum; +} + +static void create3(Params * const * pparams, int fooList[]) { + const Params * params = *pparams; + int tmpList[SIZE] = {0}; + for (int i = 0; i < params->noOfSymbols; i++) + fooList[i] = tmpList[i]; +} + +int work3(const Params * const params) { + int fooList[SIZE]; + Params *const *ptr = (Params *const*)¶ms; + create3(ptr, fooList); + int sum = 0; + for (int i = 0; i < params->noOfSymbols; i++) + sum += fooList[i]; // no-warning + return sum; +} + +typedef Params ParamsTypedef; +typedef const ParamsTypedef *ConstParamsTypedef; + +static void create4(ConstParamsTypedef const params, int fooList[]) { + int tmpList[SIZE] = {0}; + for (int i = 0; i < params->noOfSymbols; i++) + fooList[i] = tmpList[i]; +} + +int work4(Params * const params) { + int fooList[SIZE]; + create4(params, fooList); + int sum = 0; + for (int i = 0; i < params->noOfSymbols; i++) + sum += fooList[i]; // no-warning + return sum; +} diff --git a/clang/www/analyzer/available_checks.html b/clang/www/analyzer/available_checks.html index c610e2bda7332..b28fc84652352 100644 --- a/clang/www/analyzer/available_checks.html +++ b/clang/www/analyzer/available_checks.html @@ -1446,6 +1446,22 @@

Security Checkers

} + + + +