Charusso updated this revision to Diff 265962. Charusso retitled this revision from "[analyzer] CERT: STR32-C" to "[analyzer] CERT STR rule checkers: STR32-C". Charusso added a comment. Herald added subscribers: ASDenysPetrov, martong, steakhal.
- Refactor. - State out explicitly whether the Analyzer models the dynamic size. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71033/new/ https://reviews.llvm.org/D71033 Files: clang/docs/analyzer/checkers.rst clang/include/clang/StaticAnalyzer/Checkers/Checkers.td clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp clang/lib/StaticAnalyzer/Core/DynamicSize.cpp clang/test/Analysis/Inputs/system-header-simulator.h clang/test/Analysis/cert/str32-c-notes.cpp clang/test/Analysis/cert/str32-c.cpp clang/test/Analysis/malloc.c
Index: clang/test/Analysis/malloc.c =================================================================== --- clang/test/Analysis/malloc.c +++ clang/test/Analysis/malloc.c @@ -9,21 +9,6 @@ void clang_analyzer_eval(int); -// Without -fms-compatibility, wchar_t isn't a builtin type. MSVC defines -// _WCHAR_T_DEFINED if wchar_t is available. Microsoft recommends that you use -// the builtin type: "Using the typedef version can cause portability -// problems", but we're ok here because we're not actually running anything. -// Also of note is this cryptic warning: "The wchar_t type is not supported -// when you compile C code". -// -// See the docs for more: -// https://msdn.microsoft.com/en-us/library/dh8che7s.aspx -#if !defined(_WCHAR_T_DEFINED) -// "Microsoft implements wchar_t as a two-byte unsigned value" -typedef unsigned short wchar_t; -#define _WCHAR_T_DEFINED -#endif // !defined(_WCHAR_T_DEFINED) - typedef __typeof(sizeof(int)) size_t; void *malloc(size_t); void *alloca(size_t); Index: clang/test/Analysis/cert/str32-c.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/cert/str32-c.cpp @@ -0,0 +1,88 @@ +// RUN: %clang_analyze_cc1 \ +// RUN: -analyzer-checker=core,unix,alpha.security.cert.str.32c \ +// RUN: -verify %s + +// See the examples on the page of STR32-C: +// https://wiki.sei.cmu.edu/confluence/x/r9UxBQ + +#include "../Inputs/system-header-simulator.h" + +void *realloc(void *memblock, size_t size); + +namespace test_strncpy_bad { +enum { STR_SIZE = 32 }; + +size_t func(const char *source) { + char c_str[STR_SIZE]; + size_t ret = 0; + + if (source) { + c_str[sizeof(c_str) - 1] = '\0'; + strncpy(c_str, source, sizeof(c_str)); + ret = strlen(c_str); + // expected-warning@-1 {{'c_str' is not null-terminated}} + } + return ret; +} +} // namespace test_strncpy_bad + +namespace test_strncpy_good { +enum { STR_SIZE = 32 }; + +size_t func(const char *src) { + char c_str[STR_SIZE]; + size_t ret = 0; + + if (src) { + strncpy(c_str, src, sizeof(c_str) - 1); + c_str[sizeof(c_str) - 1] = '\0'; + ret = strlen(c_str); + } + return ret; +} +} // namespace test_strncpy_good + +namespace test_realloc_bad { +wchar_t *cur_msg = NULL; +size_t cur_msg_size = 1024; +size_t cur_msg_len = 0; + +void lessen_memory_usage(void) { + if (cur_msg == NULL) + return; + + size_t temp_size = cur_msg_size / 2 + 1; + wchar_t *temp = (wchar_t *)realloc(cur_msg, temp_size * sizeof(wchar_t)); + /* temp and cur_msg may no longer be null-terminated */ + if (temp == NULL) + return; + + cur_msg = temp; + cur_msg_size = temp_size; + cur_msg_len = wcslen(cur_msg); + // expected-warning@-1 {{'cur_msg' is not null-terminated}} +} +} // namespace test_realloc_bad + +namespace test_realloc_good { +wchar_t *cur_msg = NULL; +size_t cur_msg_size = 1024; +size_t cur_msg_len = 0; + +void lessen_memory_usage(void) { + if (cur_msg == NULL) + return; + + size_t temp_size = cur_msg_size / 2 + 1; + wchar_t *temp = (wchar_t *)realloc(cur_msg, temp_size * sizeof(wchar_t)); + /* temp and cur_msg may no longer be null-terminated */ + if (temp == NULL) + return; + + cur_msg = temp; + /* Properly null-terminate cur_msg */ + cur_msg[temp_size - 1] = L'\0'; + cur_msg_size = temp_size; + cur_msg_len = wcslen(cur_msg); +} +} // namespace test_realloc_good Index: clang/test/Analysis/cert/str32-c-notes.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/cert/str32-c-notes.cpp @@ -0,0 +1,38 @@ +// RUN: %clang_analyze_cc1 \ +// RUN: -analyzer-checker=core,unix,alpha.security.cert.str.32c \ +// RUN: -analyzer-output=text -verify %s + +// See the examples on the page of STR32-C: +// https://wiki.sei.cmu.edu/confluence/x/r9UxBQ + +#include "../Inputs/system-header-simulator.h" + +void *realloc(void *memblock, size_t size); + +namespace test_realloc_bad { +size_t cur_msg_size = 1024; +size_t cur_msg_len = 0; + +void lessen_memory_usage(wchar_t *cur_msg) { + if (cur_msg == NULL) { + // expected-note@-1 {{Assuming 'cur_msg' is not equal to NULL}} + // expected-note@-2 {{Taking false branch}} + return; + } + + size_t temp_size = cur_msg_size / 2 + 1; + wchar_t *temp = (wchar_t *)realloc(cur_msg, temp_size * sizeof(wchar_t)); + // expected-note@-1 {{Memory is allocated}} + // expected-note@-2 {{'temp' initialized here}} + // expected-note@-3 {{'temp' is initialized without considering the length of the string it will store, therefore it could overflow}} + + /* temp and cur_msg may no longer be null-terminated */ + + cur_msg = temp; + // expected-note@-1 {{Value assigned to 'cur_msg'}} + + cur_msg_len = wcslen(cur_msg); + // expected-note@-1 {{'cur_msg' is not null-terminated}} + // expected-warning@-2 {{'cur_msg' is not null-terminated}} +} +} // namespace test_realloc_bad Index: clang/test/Analysis/Inputs/system-header-simulator.h =================================================================== --- clang/test/Analysis/Inputs/system-header-simulator.h +++ clang/test/Analysis/Inputs/system-header-simulator.h @@ -68,8 +68,24 @@ // Note, on some platforms errno macro gets replaced with a function call. extern int errno; -size_t strlen(const char *); +// Without -fms-compatibility, wchar_t isn't a builtin type. MSVC defines +// _WCHAR_T_DEFINED if wchar_t is available. Microsoft recommends that you use +// the builtin type: "Using the typedef version can cause portability +// problems", but we're ok here because we're not actually running anything. +// Also of note is this cryptic warning: "The wchar_t type is not supported +// when you compile C code". +// +// See the docs for more: +// https://msdn.microsoft.com/en-us/library/dh8che7s.aspx +#if !defined(_WCHAR_T_DEFINED) && !__cplusplus +// "Microsoft implements wchar_t as a two-byte unsigned value" +typedef unsigned short wchar_t; +#define _WCHAR_T_DEFINED +#endif +size_t wcslen(const wchar_t *str); + +size_t strlen(const char *); char *strcpy(char *restrict, const char *restrict); char *strncpy(char *dst, const char *src, size_t n); void *memcpy(void *dst, const void *src, size_t n); Index: clang/lib/StaticAnalyzer/Core/DynamicSize.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/DynamicSize.cpp +++ clang/lib/StaticAnalyzer/Core/DynamicSize.cpp @@ -100,5 +100,17 @@ SVB); } +bool isModeledDynamicSize(ProgramStateRef State, const MemRegion *MR) { + assert(MR); + if (State->get<DynamicSizeMap>(MR)) + return true; + + if (const auto *TVR = dyn_cast<TypedValueRegion>(MR->getBaseRegion())) + if (TVR->getValueType()->getAsArrayTypeUnsafe()) + return true; + + return false; +} + } // namespace ento } // namespace clang Index: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp @@ -16,6 +16,10 @@ // character data and the null terminator: // - https://wiki.sei.cmu.edu/confluence/x/sNUxBQ // +// - STR32-C: Do not pass a non-null-terminated character sequence to a +// library function that expects a string: +// - https://wiki.sei.cmu.edu/confluence/x/r9UxBQ +// //===----------------------------------------------------------------------===// #include "AllocationState.h" @@ -27,7 +31,6 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h" -#include "llvm/ADT/Optional.h" #include <utility> using namespace clang; @@ -39,23 +42,48 @@ /// The fields are defining what is the index of the interesting arguments. struct CallContext { CallContext(Optional<unsigned> DestinationPos, - Optional<unsigned> SourcePos = None) - : DestinationPos(DestinationPos), SourcePos(SourcePos) {} + Optional<unsigned> SourcePos = None, + Optional<unsigned> LengthPos = None) + : DestinationPos(DestinationPos), SourcePos(SourcePos), + LengthPos(LengthPos) {} Optional<unsigned> DestinationPos; Optional<unsigned> SourcePos; + Optional<unsigned> LengthPos; }; -class StrCheckerBase : public Checker<check::PostCall> { +class StrCheckerBase + : public Checker<check::PostCall, check::Bind, check::PostStmt<DeclStmt>> { public: - /// Check the function calls defined in 'CDM'. + /// Check the function calls defined in 'CDM' and regular function calls. /// Function calls of 'CDM' could cause a not null-terminated string. /// /// In case of STR31-C we want to emit an error immediately on such calls. + /// + /// In case of STR32-C we want to store the string is could be not + /// null-terminated in the 'NullTerminationMap'. If such string is passed + /// to a function call we assume that the string could possibly read and + /// so that it emits an error report. void checkPostCall(const CallEvent &Call, CheckerContext &C) const; + /// Check for allocations and hand-written null-terminations (STR32-C). + /// + /// It reports a note on allocations which may cannot store a string 'S' due + /// to the string may overflow, for example the size of a memory block created + /// by 'malloc' is not based on 'strlen(S)' of 'S'. + /// + /// It catches the hand-written null-terminations, for example: + /// 'c_string[length] = '\0';'. In this case we store the string is being + /// null-terminated in the 'NullTerminationMap'. + void checkBind(SVal L, SVal V, const Stmt *S, CheckerContext &C) const; + + /// It reports a note on array declarations which may cannot store a string + /// because of its size, similar to the check in 'checkBind()'. + void checkPostStmt(const DeclStmt *S, CheckerContext &C) const; + /// \{ /// Check the function calls defined in 'CDM'. + /// STR31-C. void checkGets(const CallEvent &Call, const CallContext &CallC, CheckerContext &C) const; void checkSprintf(const CallEvent &Call, const CallContext &CallC, @@ -64,13 +92,21 @@ CheckerContext &C) const; void checkStrcpy(const CallEvent &Call, const CallContext &CallC, CheckerContext &C) const; + /// STR32-C. + void checkStrncpy(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const; /// \} - /// In case of STR31-C emit an error report of possible buffer overflow. + /// Create 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 + /// null-terminated. void createCallOverflowReport(const CallEvent &Call, const CallContext &CallC, CheckerContext &C) const; bool EnableStr31cChecker = false; + bool EnableStr32cChecker = false; private: using StrCheck = std::function<void(const StrCheckerBase *, const CallEvent &, @@ -85,13 +121,23 @@ // int fscanf(FILE *stream, const char *format, ... [char *dest]); {{"fscanf", 3, 2}, {&StrCheckerBase::checkFscanf, {2}}}, // char *strcpy(char *dest, const char *src); - {{"strcpy", 2}, {&StrCheckerBase::checkStrcpy, {0, 1}}}}; + {{"strcpy", 2}, {&StrCheckerBase::checkStrcpy, {0, 1}}}, + + // The following checks STR32-C rules. + // char *strncpy(char *dest, const char *src, size_t count) + {{"strncpy", 3}, {&StrCheckerBase::checkStrncpy, {0, 1, 2}}}}; BugType BT{this, "Insecure string handler function call", categories::SecurityError}; }; } // namespace +/// It stores whether the string is null-terminated. +REGISTER_MAP_WITH_PROGRAMSTATE(NullTerminationMap, const MemRegion *, bool) + +/// It stores the allocations which we emit notes on. +REGISTER_LIST_WITH_PROGRAMSTATE(AllocationList, const MemRegion *) + //===----------------------------------------------------------------------===// // Helper functions. //===----------------------------------------------------------------------===// @@ -125,27 +171,189 @@ if (!MR) return; - ProgramStateRef State = C.getState(); - StringRef CallName = Call.getCalleeIdentifier()->getName(); - const Expr *Arg = Call.getArgExpr(DestPos); SmallString<256> Msg; llvm::raw_svector_ostream Out(Msg); Out << '\'' << CallName << "' could write outside of '" << printPretty(Arg, C) << '\''; + std::string ReportMessage = Out.str().str(); - auto Report = std::make_unique<PathSensitiveBugReport>(BT, Out.str(), - C.generateErrorNode()); - Report->addRange(Arg->getSourceRange()); + // In case of STR31-C emit an error. + if (EnableStr31cChecker) { + const Expr *Arg = Call.getArgExpr(DestPos); - // Track the allocation. - bugreporter::trackExpressionValue(Report->getErrorNode(), Arg, *Report); - if (const SymbolRef Sym = DestV.getAsSymbol()) - Report->addVisitor(allocation_state::getMallocBRVisitor(Sym)); + auto Report = std::make_unique<PathSensitiveBugReport>( + BT, ReportMessage, C.generateErrorNode()); + Report->addRange(Arg->getSourceRange()); + Report->markInteresting(MR); - C.emitReport(std::move(Report)); + // Track the allocation. + bugreporter::trackExpressionValue(Report->getErrorNode(), Arg, *Report); + if (const SymbolRef Sym = DestV.getAsSymbol()) + Report->addVisitor(allocation_state::getMallocBRVisitor(Sym)); + + C.emitReport(std::move(Report)); + return; + } + + // In case of STR32-C emit a note. + if (!EnableStr32cChecker) + return; + + const NoteTag *Tag = C.getNoteTag( + [=](PathSensitiveBugReport &BR) -> std::string { + return BR.isInteresting(MR) ? ReportMessage : ""; + }, + /*IsPrunable=*/false); + + // Mark the string could be not null-terminated. + ProgramStateRef State = C.getState(); + State = State->set<NullTerminationMap>(MR, false); + C.addTransition(State, Tag); +} + +/// Create a note about the buffer could overflow and store the allocation +/// may cannot store the entire null-terminated string. +static void createAllocationOverflowReport(const MemRegion *MR, + Optional<std::string> ArrayName, + bool IsInit, CheckerContext &C) { + ProgramStateRef State = C.getState(); + const NoteTag *Tag = C.getNoteTag( + [=](PathSensitiveBugReport &BR) -> std::string { + if (!BR.isInteresting(MR)) + return ""; + + SmallString<128> Msg; + llvm::raw_svector_ostream Out(Msg); + Out << (ArrayName ? '\'' + *ArrayName + '\'' : "The array") << " is " + << (IsInit ? "initialized" : "declared") + << " without considering the length of the string it " + "will store, therefore it could overflow"; + + return Out.str().str(); + }, + /*IsPrunable=*/false); + + // Mark the string it will store could be not null-terminated. + State = State->set<NullTerminationMap>(MR, false); + State = State->add<AllocationList>(MR); + C.addTransition(State, Tag); +} + +/// Check whether the size of the memory block allocated is based on the string +/// it will store. +// FIXME: Use better heuristics to catch such constructs. +// FIXME: Make sure the size is appropriate for the null-terminator. +static bool isWrongSizeAllocation(const MemRegion *MR, CheckerContext &C) { + // Only check for allocations we model. + ProgramStateRef State = C.getState(); + if (!isModeledDynamicSize(State, MR)) + return false; + + SValBuilder &SVB = C.getSValBuilder(); + DefinedOrUnknownSVal AllocationSize = getDynamicSize(State, MR, SVB); + + // 'strlen(something) + something' is most likely fine. + if (const SymExpr *SizeSym = AllocationSize.getAsSymExpr()) + if (const auto *SIE = dyn_cast<SymIntExpr>(SizeSym)) + if (SIE->getOpcode() == BO_Add) + if (const auto *SM = dyn_cast<SymbolMetadata>(SIE->getLHS())) + return false; + + return true; +} + +/// Check whether the function call could write outside of the buffer. +static bool isCallOverflow(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) { + SVal DestV = Call.getArgSVal(*CallC.DestinationPos); + SVal SrcV = Call.getArgSVal(*CallC.SourcePos); + + ProgramStateRef State = C.getState(); + SValBuilder &SVB = C.getSValBuilder(); + + // Check the size of the allocation to prevent false alarms. + const MemRegion *SrcMR = getRegion(SrcV); + const MemRegion *DestMR = getRegion(DestV); + if (!SrcMR || !DestMR) + return false; + + // Only check for allocations we model. + if (!isModeledDynamicSize(State, DestMR)) + return false; + + DefinedOrUnknownSVal SrcSize = getDynamicSize(State, SrcMR, SVB); + DefinedOrUnknownSVal DestSize = getDynamicSize(State, DestMR, SVB); + + if (SrcSize == DestSize) + return false; + + if (const llvm::APSInt *SrcSizeInt = SVB.getKnownValue(State, SrcSize)) + if (const llvm::APSInt *DestSizeInt = SVB.getKnownValue(State, DestSize)) + if (SrcSizeInt->getZExtValue() <= DestSizeInt->getZExtValue()) + return false; + + if (!isWrongSizeAllocation(DestMR, C)) + return false; + + return true; +} + +static bool isNonConstCharAllocation(const MemRegion *MR) { + const auto *TVR = MR->getAs<TypedValueRegion>(); + if (!TVR) + return false; + + QualType Ty = TVR->getValueType(); + if (Ty.isNull()) + return false; + + while (Ty->isPointerType()) + Ty = Ty->getPointeeType(); + + if (const ArrayType *AT = Ty->getAsArrayTypeUnsafe()) + Ty = AT->getElementType(); + + return Ty->isAnyCharacterType() && !Ty.isConstQualified(); +} + +/// Check whether the memory allocation could overflow, if so emit a report. +static bool isAllocationOverflow(SVal L, SVal V, const Stmt *S, + CheckerContext &C) { + const MemRegion *LocMR = getRegion(L); + const MemRegion *NonLocMR = getRegion(V); + if (!LocMR || !NonLocMR) + return false; + + // A C-string is null-terminated. + if (isa<StringRegion>(NonLocMR)) + return false; + + if (!isNonConstCharAllocation(LocMR)) + return false; + + // See whether we have already made a report for that allocation, so it + // prevent additional reports on rebindings. + ProgramStateRef State = C.getState(); + if (State->get<AllocationList>().contains(NonLocMR)) + return false; + + if (!isWrongSizeAllocation(NonLocMR, C)) + return false; + + bool IsInit = false; + Optional<std::string> ArrayName; + if (const auto *VR = LocMR->getAs<VarRegion>()) { + if (const VarDecl *VD = VR->getDecl()) { + IsInit = VD->getInit(); + ArrayName = VD->getNameAsString(); + } + } + + createAllocationOverflowReport(NonLocMR, ArrayName, IsInit, C); + return true; } //===----------------------------------------------------------------------===// @@ -186,46 +394,60 @@ void StrCheckerBase::checkStrcpy(const CallEvent &Call, const CallContext &CallC, CheckerContext &C) const { - if (!EnableStr31cChecker) + if (EnableStr31cChecker || EnableStr32cChecker) + if (isCallOverflow(Call, CallC, C)) + createCallOverflowReport(Call, CallC, C); +} + +//===----------------------------------------------------------------------===// +// The following checks STR32-C rules. +//===----------------------------------------------------------------------===// + +void StrCheckerBase::checkStrncpy(const CallEvent &Call, + const CallContext &CallC, + CheckerContext &C) const { + if (EnableStr32cChecker) + if (isCallOverflow(Call, CallC, C)) + createCallOverflowReport(Call, CallC, C); +} + +void StrCheckerBase::checkBind(SVal L, SVal V, const Stmt *S, + CheckerContext &C) const { + if (!EnableStr32cChecker) + return; + + if (isAllocationOverflow(L, V, S, C)) return; ProgramStateRef State = C.getState(); - SValBuilder &SVB = C.getSValBuilder(); - SVal SrcV = Call.getArgSVal(*CallC.SourcePos); - SVal DestV = Call.getArgSVal(*CallC.DestinationPos); + bool IsNullTermination = false; + if (const llvm::APInt *Int = C.getSValBuilder().getKnownValue(State, V)) + IsNullTermination = Int->isNullValue(); - // Check the size of the allocation 'DestSize' to prevent false alarms. - const MemRegion *SrcMR = getRegion(SrcV); - const MemRegion *DestMR = getRegion(DestV); - if (!SrcMR || !DestMR) + if (!IsNullTermination) return; - DefinedOrUnknownSVal SrcSize = getDynamicSize(State, SrcMR, SVB); - DefinedOrUnknownSVal DestSize = getDynamicSize(State, DestMR, SVB); + // Mark the string null-terminated. + State = State->set<NullTerminationMap>(getRegion(L), true); + C.addTransition(State); +} - // Only care about allocations we could model. - // FIXME: Use heuristics what is an allocator and try to obtain its size. - if (DestSize.isUnknown()) +void StrCheckerBase::checkPostStmt(const DeclStmt *S, CheckerContext &C) const { + if (!EnableStr32cChecker) return; - if (SrcSize == DestSize) - return; + if (const auto *VD = cast<VarDecl>(S->getSingleDecl())) { + if (VD->getType()->getAsArrayTypeUnsafe()) { + ProgramStateRef State = C.getState(); - if (const llvm::APSInt *SrcSizeInt = SVB.getKnownValue(State, SrcSize)) - if (const llvm::APSInt *DestSizeInt = SVB.getKnownValue(State, DestSize)) - if (SrcSizeInt->getZExtValue() <= DestSizeInt->getZExtValue()) + const VarRegion *VR = State->getRegion(VD, C.getLocationContext()); + if (!isNonConstCharAllocation(VR)) return; - // Allocating size of 'strlen(src)' is most likely fine. - // FIXME: Make sure we catch 'strlen()' in complex SVals. - // FIXME: Calcualte the appropriate size for holding the string. - if (const SymExpr *SE = DestSize.getAsSymExpr()) - if (const auto *SIE = dyn_cast<SymIntExpr>(SE)) - if (const auto *SM = dyn_cast<SymbolMetadata>(SIE->getLHS())) - if (SM->getRegion() == SrcMR) - return; - - createCallOverflowReport(Call, CallC, C); + std::string ArrayName = VD->getNameAsString(); + createAllocationOverflowReport(VR, ArrayName, VD->getInit(), C); + } + } } //===----------------------------------------------------------------------===// @@ -237,6 +459,51 @@ if (const auto *Lookup = CDM.lookup(Call)) { const StrCheck &Check = Lookup->first; Check(this, Call, Lookup->second, C); + return; + } + + // In case of STR32-C we want to catch not null-terminated strings passed + // to a function call. + if (!EnableStr32cChecker) + return; + + // If the passed parameter is a const-qualified string we assume that the + // string could be read. If such string is not null-terminated emit a report. + for (unsigned I = 0; I < Call.parameters().size(); ++I) { + SVal V = Call.getArgSVal(I); + const MemRegion *MR = getRegion(V); + if (!MR) + continue; + + QualType ParamTy = Call.parameters()[I]->getType(); + if (ParamTy->isPointerType()) + ParamTy = ParamTy->getPointeeType(); + + if (!ParamTy.isConstQualified()) + continue; + + ProgramStateRef State = C.getState(); + const bool *IsNullTerminated = State->get<NullTerminationMap>(MR); + if (!IsNullTerminated || *IsNullTerminated) + continue; + + SmallString<128> Msg; + llvm::raw_svector_ostream Out(Msg); + + const Expr *Arg = Call.getArgExpr(I); + Out << '\'' << printPretty(Arg, C) << "' is not null-terminated"; + + auto Report = std::make_unique<PathSensitiveBugReport>( + BT, Out.str(), C.generateErrorNode()); + Report->addRange(Arg->getSourceRange()); + Report->markInteresting(MR); + + // Track the allocation. + bugreporter::trackExpressionValue(Report->getErrorNode(), Arg, *Report); + if (const SymbolRef Sym = Call.getArgSVal(I).getAsSymbol()) + Report->addVisitor(allocation_state::getMallocBRVisitor(Sym)); + + C.emitReport(std::move(Report)); } } @@ -255,3 +522,4 @@ bool ento::shouldRegister##Name(const CheckerManager &) { return true; } REGISTER_CHECKER(Str31cChecker) +REGISTER_CHECKER(Str32cChecker) Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h @@ -43,6 +43,9 @@ const CXXNewExpr *NE, const LocationContext *LCtx, SValBuilder &SVB); +/// \returns Whether we model the size explicitly. +bool isModeledDynamicSize(ProgramStateRef State, const MemRegion *MR); + } // namespace ento } // namespace clang Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -861,6 +861,11 @@ Dependencies<[StrCheckerBase]>, Documentation<HasDocumentation>; +def Str32cChecker : Checker<"32c">, + HelpText<"SEI CERT checker of rules defined in STR32-C">, + Dependencies<[StrCheckerBase]>, + Documentation<HasDocumentation>; + } // end "alpha.cert.str" let ParentPackage = SecurityAlpha in { Index: clang/docs/analyzer/checkers.rst =================================================================== --- clang/docs/analyzer/checkers.rst +++ clang/docs/analyzer/checkers.rst @@ -1971,6 +1971,18 @@ It warns on misusing the following functions: ``strcpy()``, ``gets()``, ``fscanf()``, ``sprintf()``. +.. _alpha-security-cert-str-32c: + +alpha.security.cert.str.32c +""""""""""""""""""""""""""" + +Checker for `STR32-C rule<https://wiki.sei.cmu.edu/confluence/x/r9UxBQ>`_. + +It warns on reading non-null-terminated strings. This warning is restricted to +the allocations which the Static Analyzer models with :ref:`unix.Malloc<unix-Malloc>`_. + +Also warns on misusing the ``strncpy()`` function. + .. _alpha-security-ArrayBound: alpha.security.ArrayBound (C)
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
