Index: clang/docs/analyzer/checkers.rst =================================================================== --- clang/docs/analyzer/checkers.rst +++ clang/docs/analyzer/checkers.rst @@ -2003,6 +2003,15 @@ It warns on misusing ``std::cin`` and ``std::basic_istream::read()`` to create a ``std::string`` which may not null-terminated. +.. _alpha-security-cert-str-51cpp: + +alpha.security.cert.str.51cpp +""""""""""""""""""""""""""""" + +Checker for `STR51-CPP rule`_. + +It warns on possible ``std::string`` construction from a ``nullptr``. + .. _alpha-security-ArrayBound: alpha.security.ArrayBound (C) Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -876,6 +876,11 @@ Dependencies<[StrCheckerBase]>, Documentation; +def Str51cppChecker : Checker<"51cpp">, + HelpText<"SEI CERT checker of rules defined in STR51-CPP">, + Dependencies<[StrCheckerBase]>, + Documentation; + } // end "alpha.cert.str" let ParentPackage = SecurityAlpha in { Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h @@ -64,6 +64,8 @@ return Eng.getStoreManager(); } + MemRegionManager &getRegionManager() { return Eng.getRegionManager(); } + /// Returns the previous node in the exploded graph, which includes /// the state of the program before the checker ran. Note, checkers should /// not retain the node in their state since the nodes might get invalidated. Index: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp @@ -27,6 +27,9 @@ // character data and the null terminator: // - https://wiki.sei.cmu.edu/confluence/x/i3w-BQ // +// - STR51-CPP: Do not attempt to create a std::string from a null pointer: +// - https://wiki.sei.cmu.edu/confluence/x/E3s-BQ +// //===----------------------------------------------------------------------===// #include "AllocationState.h" @@ -90,6 +93,9 @@ /// because of its size, similar to the check in 'checkBind()'. void checkPostStmt(const DeclStmt *S, CheckerContext &C) const; + /// Evalute function calls defined in 'EvalCDM'. + bool evalCall(const CallEvent &Call, CheckerContext &C) const; + /// \{ /// Check the function calls defined in 'CDM'. /// STR30-C. @@ -124,7 +130,7 @@ CheckerContext &C) const; /// \} - /// Create a report on a function call which could cause a buffer overflow. + /// Creates a report on a function call which could cause a buffer overflow. /// /// In case of STR31-C emit an error report. /// In case of STR32-C emit a note and mark the buffer as possibly not @@ -136,6 +142,7 @@ bool EnableStr31cChecker = false; bool EnableStr32cChecker = false; bool EnableStr50cppChecker = false; + bool EnableStr51cppChecker = false; private: using StrCheck = std::function(getRegion(Call.getArgSVal(0))); - if (!DR) + + ProgramStateRef State = C.getState(); + const Expr *SrcArg = Call.getArgExpr(0); + + // In case of STR51-CPP we need to be sure whether the constructor is + // called with a non-null string. + SVal SrcV = Call.getArgSVal(0); + if (EnableStr51cppChecker && MR) { + // We are only interested in possible nullptrs. + // Assume that only non-arguments could be nullable. + // Assume that only non-methods could be nullable. + // FIXME: Add more assumptions to tweak the reports. + bool IsNullable = false; + if (DR) { + IsNullable = !isa(DR->getMemorySpace()); + } else if (const auto *CE = dyn_cast(SrcArg->IgnoreImpCasts())) { + IsNullable = !dyn_cast(CE); + } + + // 'SrcV' is problematic iff it is not exactly non-null and nullable. + if (IsNullable && MR->getSymbolicBase() && + !State->isNonNull(SrcV).isConstrainedTrue()) { + + auto Report = std::make_unique( + BT, "Constructing from nullptr", C.generateErrorNode()); + Report->addRange(SrcArg->getSourceRange()); + Report->markInteresting(MR); + + // Track the allocation. + bugreporter::trackExpressionValue(Report->getErrorNode(), SrcArg, + *Report); + if (const SymbolRef Sym = C.getSVal(SrcArg).getAsSymbol()) + Report->addVisitor(allocation_state::getMallocBRVisitor(Sym)); + + C.emitReport(std::move(Report)); + return; + } + } + + // In case of STR50-CPP we need to be sure whether the constructor is + // called with a null-terminated string. + if (!EnableStr50cppChecker) return; - const bool *IsNullTerminated = C.getState()->get(DR); + // If the second argument is present which is the size of the string + // assume that the string will be null-terminated. + if (Call.getNumArgs() > 1) + return; + + const bool *IsNullTerminated = State->get(MR); if (!IsNullTerminated || *IsNullTerminated) return; @@ -740,7 +796,6 @@ auto Report = std::make_unique(BT, Out.str(), C.generateErrorNode()); - const Expr *SrcArg = Call.getArgExpr(0); Report->addRange(SrcArg->getSourceRange()); Report->markInteresting(DR); @@ -827,3 +882,4 @@ REGISTER_CHECKER(Str31cChecker) REGISTER_CHECKER(Str32cChecker) REGISTER_CHECKER(Str50cppChecker) +REGISTER_CHECKER(Str51cppChecker) Index: clang/test/Analysis/Inputs/system-header-simulator-cxx.h =================================================================== --- clang/test/Analysis/Inputs/system-header-simulator-cxx.h +++ clang/test/Analysis/Inputs/system-header-simulator-cxx.h @@ -580,6 +580,9 @@ void resize(size_type count); void shrink_to_fit(); void swap(basic_string &other); + + bool empty(); + size_t size() const; }; typedef basic_string string; Index: clang/test/Analysis/cert/str51-cpp-false-positive-suppression.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/cert/str51-cpp-false-positive-suppression.cpp @@ -0,0 +1,33 @@ +// RUN: %clang_analyze_cc1 -fcxx-exceptions \ +// RUN: -analyzer-checker=core,unix,alpha.security.cert.str.51cpp \ +// RUN: -verify %s + +// See the examples on the page of STR51-CPP: +// https://wiki.sei.cmu.edu/confluence/x/E3s-BQ + +// expected-no-diagnostics + +#include "../Inputs/system-header-simulator.h" +#include "../Inputs/system-header-simulator-cxx.h" + +namespace std { +char *getenv(const char *varname); +}; // namespace std + +class C { + std::string S; +public: + C(const std::string &S) : S(S) {} // no-warning: StackArgumentsSpaceRegion +}; + +class Slice { +public: + Slice() : data_(""), size_(0) { } + + std::string ToString() const { return std::string(data_, size_); } + // no-warning: StackArgumentsSpaceRegion + +private: + const char* data_; + size_t size_; +}; Index: clang/test/Analysis/cert/str51-cpp.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/cert/str51-cpp.cpp @@ -0,0 +1,32 @@ +// RUN: %clang_analyze_cc1 -fcxx-exceptions \ +// RUN: -analyzer-checker=core,unix,alpha.security.cert.str.51cpp \ +// RUN: -verify %s + +// See the examples on the page of STR51-CPP: +// https://wiki.sei.cmu.edu/confluence/x/E3s-BQ + +#include "../Inputs/system-header-simulator-cxx.h" + +namespace std { +char *getenv(const char *varname); +}; // namespace std + +namespace test_getenv_bad { +void f() { + std::string tmp(std::getenv("TMP")); + // expected-warning@-1 {{Constructing from nullptr}} + if (!tmp.empty()) { + // ... + } +} +} // namespace test_getenv_bad + +namespace test_getenv_good { +void f() { + const char *tmpPtrVal = std::getenv("TMP"); + std::string tmp(tmpPtrVal ? tmpPtrVal : ""); + if (!tmp.empty()) { + // ... + } +} +} // namespace test_getenv_good Index: clang/test/Analysis/diagnostics/explicit-suppression.cpp =================================================================== --- clang/test/Analysis/diagnostics/explicit-suppression.cpp +++ clang/test/Analysis/diagnostics/explicit-suppression.cpp @@ -19,6 +19,6 @@ void testCopyNull(C *I, C *E) { std::copy(I, E, (C *)0); #ifndef SUPPRESSED - // expected-warning@../Inputs/system-header-simulator-cxx.h:722 {{Called C++ object pointer is null}} + // expected-warning@../Inputs/system-header-simulator-cxx.h:725 {{Called C++ object pointer is null}} #endif }