Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, mgorny.
This checker warns on most of the following rules: https://wiki.sei.cmu.edu/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator Repository: rC Clang https://reviews.llvm.org/D70411 Files: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSizeInfo.h clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h clang/lib/StaticAnalyzer/Checkers/AllocationState.h clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp clang/lib/StaticAnalyzer/Core/BugReporter.cpp clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp clang/test/Analysis/Inputs/system-header-simulator.h clang/test/Analysis/cert/str31-c-fp-suppression.cpp clang/test/Analysis/cert/str31-c-notes.cpp clang/test/Analysis/cert/str31-c.cpp
Index: clang/test/Analysis/cert/str31-c.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/cert/str31-c.cpp @@ -0,0 +1,181 @@ +// RUN: %clang_analyze_cc1 \ +// RUN: -analyzer-checker=core,unix,security.cert.str.31.c \ +// RUN: -verify %s + +// See the examples on the page of STR31: +// https://wiki.sei.cmu.edu/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator + +#include "../Inputs/system-header-simulator.h" + +#define EOF -1 +typedef __SIZE_TYPE__ size_t; + +void free(void *memblock); +void *malloc(size_t size); + +namespace test_gets_bad { +#define BUFFER_SIZE 1024 + +void func(void) { + char buf[BUFFER_SIZE]; + if (gets(buf)) {} + // expected-warning@-1 {{'gets' could write outside of 'buf'}} +} +} // namespace test_gets_bad + +namespace test_gets_good { +enum { BUFFERSIZE = 32 }; + +void func(void) { + char buff[BUFFERSIZE]; + + if (fgets(buff, sizeof(buff), stdin)) {} +} +} // namespace test_gets_good + +namespace test_sprintf_bad { +void func(const char *name) { + char buf[128]; + sprintf(buf, "%s.txt", name); + // expected-warning@-1 {{'sprintf' could write outside of 'buf'}} +} +} // namespace test_sprintf_bad + +namespace test_sprintf_good { +void func(const char *name) { + char buff[128]; + snprintf(buff, sizeof(buff), "%s.txt", name); +} +} // namespace test_sprintf_good + +namespace test_fscanf_bad { +enum { BUF_LENGTH = 1024 }; + +void get_data(void) { + char buf[BUF_LENGTH]; + if (fscanf(stdin, "%s", buf)) {} + // expected-warning@-1 {{'fscanf' could write outside of 'buf'}} +} +} // namespace test_fscanf_bad + +namespace test_fscanf_good { +enum { BUF_LENGTH = 1024 }; + +void get_data(void) { + char buff[BUF_LENGTH]; + if (fscanf(stdin, "%1023s", buff)) {} +} +} // namespace test_fscanf_good + +namespace test_strcpy_bad { +int main(int argc, char *argv[]) { + const char *const name = (argc && argv[0]) ? argv[0] : ""; + char prog_name[128]; + strcpy(prog_name, name); + // expected-warning@-1 {{'strcpy' could write outside of 'prog_name'}} + return 0; +} + +void func(void) { + char buff[256]; + char *editor = getenv("EDITOR"); + if (editor != NULL) { + strcpy(buff, editor); + // expected-warning@-1 {{'strcpy' could write outside of 'buff'}} + } +} +} // namespace test_strcpy_bad + +namespace test_strcpy_good { +int main(int argc, char *argv[]) { + const char *const name = (argc && argv[0]) ? argv[0] : ""; + char *prog_name2 = (char *)malloc(strlen(name) + 1); + if (prog_name2 != NULL) { + strcpy(prog_name2, name); + } + free(prog_name2); + return 0; +} + +void func(void) { + char *buff2; + char *editor = getenv("EDITOR"); + if (editor != NULL) { + size_t len = strlen(editor) + 1; + buff2 = (char *)malloc(len); + if (buff2 != NULL) { + strcpy(buff2, editor); + } + free(buff2); + } +} +} // namespace test_strcpy_good + +//===----------------------------------------------------------------------===// +// The following is from the rule's page which we do not handle yet. +//===----------------------------------------------------------------------===// + +namespace test_loop_index_bad { +void copy(size_t n, char src[n], char dest[n]) { + size_t i; + + for (i = 0; src[i] && (i < n); ++i) { + dest[i] = src[i]; + } + dest[i] = '\0'; +} +} // namespace test_loop_index_bad + +namespace test_loop_index_good { +void copy(size_t n, char src[n], char dest[n]) { + size_t i; + + for (i = 0; src[i] && (i < n - 1); ++i) { + dest[i] = src[i]; + } + dest[i] = '\0'; +} +} // namespace test_loop_index_good + +namespace test_getchar_bad { +enum { BUFFERSIZE = 32 }; + +void func(void) { + char buf[BUFFERSIZE]; + char *p; + int ch; + p = buf; + while ((ch = getchar()) != '\n' && ch != EOF) { + *p++ = (char)ch; + } + *p++ = 0; + if (ch == EOF) { + /* Handle EOF or error */ + } +} +} // namespace test_getchar_bad + +namespace test_getchar_good { +enum { BUFFERSIZE = 32 }; + +void func(void) { + char buf[BUFFERSIZE]; + int ch; + size_t index = 0; + size_t chars_read = 0; + + while ((ch = getchar()) != '\n' && ch != EOF) { + if (index < sizeof(buf) - 1) { + buf[index++] = (char)ch; + } + chars_read++; + } + buf[index] = '\0'; /* Terminate string */ + if (ch == EOF) { + /* Handle EOF or error */ + } + if (chars_read > index) { + /* Handle truncation */ + } +} +} // namespace test_getchar_good Index: clang/test/Analysis/cert/str31-c-notes.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/cert/str31-c-notes.cpp @@ -0,0 +1,37 @@ +// RUN: %clang_analyze_cc1 \ +// RUN: -analyzer-checker=core,unix,security.cert.str.31.c \ +// RUN: -analyzer-output=text -verify %s + +// See the examples on the page of STR31: +// https://wiki.sei.cmu.edu/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator + +#include "../Inputs/system-header-simulator.h" + +void *malloc(size_t size); +void free(void *memblock); + +void test_simple_size(unsigned size) { + size = 13; + + char *buf = (char *)malloc(size); + // expected-note@-1 {{Memory is allocated}} + + if (gets(buf)) {} + // expected-warning@-1 {{'gets' could write outside of 'buf'}} + // expected-note@-2 {{'gets' could write outside of 'buf'}} + free(buf); +} + +void test_size_redefinition() { + unsigned size = 13; + + char *buf = (char *)malloc(size + 1); + // expected-note@-1 {{Memory is allocated}} + + size = 42; + + if (gets(buf)) {} + // expected-warning@-1 {{'gets' could write outside of 'buf'}} + // expected-note@-2 {{'gets' could write outside of 'buf'}} + free(buf); +} Index: clang/test/Analysis/cert/str31-c-fp-suppression.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/cert/str31-c-fp-suppression.cpp @@ -0,0 +1,78 @@ +// RUN: %clang_analyze_cc1 \ +// RUN: -analyzer-checker=core,unix,security.cert.str.31.c \ +// RUN: -verify %s + +// expected-no-diagnostics + +// See the examples on the page of STR31: +// https://wiki.sei.cmu.edu/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator + +#include "../Inputs/system-header-simulator.h" + +#define EOF -1 + +void free(void *memblock); +void *malloc(size_t size); + +void do_something(char *buffer, size_t size); +bool coin(); + +void test_wonky_null_termination(const char *src) { + char dest[128]; + + // FIXME: We need to model more about string manipulation because may some + // of the warnings are not false positives. For now assume that the explicit + // null-termination handles the string well. + strcpy(dest, src); // no-warning + + strcpy(dest, src); // no-warning + dest[sizeof(dest) - 1] = '\0'; +} + +void test_may_not_every_path_null_terminated(const char *src) { + char dest[128]; + strcpy(dest, src); // no-warning + + // FIXME: We need to model more about the dynamic element count of the + // string, so that we could see whether the string is not null-terminated + // on a path. For now assume that the intention of explicit null-termination + // most likely leads to a correct string. + if (strlen(src) >= sizeof(dest)) { + dest[sizeof(dest) - 1] = '\0'; + } +} + +void test_using_before_terminating(const char *src) { + char dest[128]; + strcpy(dest, src); + + // FIXME: Emit an error when the null-termination happen later than any + // usage of possible not null-terminated the C-string. + do_something(dest, 128); + *dest = '\0'; +} + +// FIXME: The following tests could fail if the destination array has an offset. + +void test_known_size_cannot_overflow() { + char dest[128]; + strcpy(dest, "src"); +} + +void test_everything_known() { + char *dest2 = (char *)malloc(strlen("Foo") + 1); + if (dest2 != NULL) { + strcpy(dest2, "Foo"); + } + free(dest2); +} + +void test_multiply_on_size(const char *src) { + char *buff2; + size_t len = strlen(src) + 1; + buff2 = (char *)malloc(2 * len); + if (buff2 != NULL) { + strcpy(buff2, src); + } + free(buff2); +} 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 @@ -18,22 +18,32 @@ extern FILE *__stdoutp; extern FILE *__stderrp; +typedef __SIZE_TYPE__ size_t; + int scanf(const char *restrict format, ...); -int fscanf(FILE *restrict, const char *restrict, ...); int printf(const char *restrict format, ...); int fprintf(FILE *restrict, const char *restrict, ...); int getchar(void); +char *gets(char *buffer); +char *gets_s(char *buffer, size_t size); +char *fgets(char *str, int n, FILE *stream); +int sprintf(char *buffer, const char *format, ...); +int snprintf(char *buffer, size_t count, const char *format, ...); +int _snprintf_s(char *buffer, size_t sizeOfBuffer, size_t count, const char *format, ...); +int fscanf(FILE *stream, const char *format, ...); +int fscanf_s(FILE *stream, const char *format, ...); +char *getenv(const char *varname); // Note, on some platforms errno macro gets replaced with a function call. extern int errno; - -typedef __typeof(sizeof(int)) size_t; +typedef int errno_t; size_t strlen(const char *); +char *strcpy(char *dest, const char *src); +errno_t strcpy_s(char *dest, size_t destSize, const char *src); +char *strncpy(char *dest, const char *src, size_t count); -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); +void *memcpy(void *dest, const void *src, size_t count); typedef unsigned long __darwin_pthread_key_t; typedef __darwin_pthread_key_t pthread_key_t; @@ -112,4 +122,4 @@ #define NULL __DARWIN_NULL #endif -#define offsetof(t, d) __builtin_offsetof(t, d) \ No newline at end of file +#define offsetof(t, d) __builtin_offsetof(t, d) Index: clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp +++ clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp @@ -9,13 +9,19 @@ #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h" // Common strings used for the "category" of many static analyzer issues. -namespace clang { namespace ento { namespace categories { +namespace clang { +namespace ento { +namespace categories { -const char * const CoreFoundationObjectiveC = "Core Foundation/Objective-C"; -const char * const LogicError = "Logic error"; -const char * const MemoryRefCount = - "Memory (Core Foundation/Objective-C/OSObject)"; -const char * const MemoryError = "Memory error"; -const char * const UnixAPI = "Unix API"; -const char * const CXXObjectLifecycle = "C++ object lifecycle"; -}}} +const char *const CoreFoundationObjectiveC = "Core Foundation/Objective-C"; +const char *const LogicError = "Logic error"; +const char *const MemoryRefCount = + "Memory (Core Foundation/Objective-C/OSObject)"; +const char *const MemoryError = "Memory error"; +const char *const UnixAPI = "Unix API"; +const char *const CXXObjectLifecycle = "C++ object lifecycle"; +const char *const SecurityError = "SecurityError"; + +} // namespace categories +} // namespace ento +} // namespace clang Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/BugReporter.cpp +++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -2484,11 +2484,20 @@ BugPathGetter::BugPathGetter(const ExplodedGraph *OriginalGraph, ArrayRef<PathSensitiveBugReport *> &bugReports) { + + bool HasValidReport = false; + for (const auto I : bugReports) { + if (I->isValid()) { + HasValidReport = true; + break; + } + } + + if (!HasValidReport) + return; + SmallVector<const ExplodedNode *, 32> Nodes; for (const auto I : bugReports) { - assert(I->isValid() && - "We only allow BugReporterVisitors and BugReporter itself to " - "invalidate reports!"); Nodes.emplace_back(I->getErrorNode()); } @@ -2756,7 +2765,9 @@ // Find the BugReport with the original location. PathSensitiveBugReport *R = BugPath->Report; assert(R && "No original report found for sliced graph."); - assert(R->isValid() && "Report selected by trimmed graph marked invalid."); + if (!R->isValid()) + continue; + const ExplodedNode *ErrorNode = BugPath->ErrorNode; // Register refutation visitors first, if they mark the bug invalid no @@ -2774,23 +2785,22 @@ std::unique_ptr<VisitorsDiagnosticsTy> visitorNotes = generateVisitorsDiagnostics(R, ErrorNode, BRC); - if (R->isValid()) { - if (Reporter.getAnalyzerOptions().ShouldCrosscheckWithZ3) { - // If crosscheck is enabled, remove all visitors, add the refutation - // visitor and check again - R->clearVisitors(); - R->addVisitor(std::make_unique<FalsePositiveRefutationBRVisitor>()); - - // We don't overrite the notes inserted by other visitors because the - // refutation manager does not add any new note to the path - generateVisitorsDiagnostics(R, BugPath->ErrorNode, BRC); - } + if (Reporter.getAnalyzerOptions().ShouldCrosscheckWithZ3) { + // If crosscheck is enabled, remove all visitors, add the refutation + // visitor and check again + R->clearVisitors(); + R->addVisitor(std::make_unique<FalsePositiveRefutationBRVisitor>()); + + // We don't overrite the notes inserted by other visitors because the + // refutation manager does not add any new note to the path + generateVisitorsDiagnostics(R, BugPath->ErrorNode, BRC); + } - // Check if the bug is still valid - if (R->isValid()) - return PathDiagnosticBuilder( - std::move(BRC), std::move(BugPath->BugPath), BugPath->Report, - BugPath->ErrorNode, std::move(visitorNotes)); + // Check if the bug is still valid + if (R->isValid()) { + return PathDiagnosticBuilder(std::move(BRC), std::move(BugPath->BugPath), + BugPath->Report, BugPath->ErrorNode, + std::move(visitorNotes)); } } Index: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp =================================================================== --- /dev/null +++ clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp @@ -0,0 +1,309 @@ +//===- StrChecker - SEI CERT checker of rules defined in STR ----*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file defines StrChecker which tries to find the SEI CERT string +// handler function issues. +// The rules can be found in section 'Rule 07. Characters and Strings (STR)': +// https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152038 +// +// This checker is a base checker which consist of the following checkers: +// - '31.c' +// https://wiki.sei.cmu.edu/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator +// +//===----------------------------------------------------------------------===// + +#include "AllocationState.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/CheckerManager.h" +#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; +using namespace ento; +using namespace ast_matchers; + +namespace { + +struct CallContext { + CallContext(Optional<unsigned> DestinationPos, + Optional<unsigned> SourcePos = None) + : DestinationPos(DestinationPos), SourcePos(SourcePos) {} + + Optional<unsigned> DestinationPos; + Optional<unsigned> SourcePos; +}; + +class StrChecker : public Checker<eval::Call, check::Bind> { + using StrCheck = std::function<void(const StrChecker *, const CallEvent &, + const CallContext &, CheckerContext &)>; + +public: + bool evalCall(const CallEvent &Call, CheckerContext &C) const; + // We want to catch the hand-written null termination of C-strings to prevent + // false positives. For example: 'c_string[length] = '\0';' + void checkBind(SVal L, SVal V, const Stmt *S, CheckerContext &C) const; + + std::unique_ptr<BugType> BT; + bool EnableStr31cChecker = false; + + const CallDescriptionMap<std::pair<StrCheck, CallContext>> CDM = { + // The following checks STR31-C rules. + // char *gets(char *dest); + {{"gets", 1}, {&StrChecker::evalGets, {0}}}, + // int sprintf(char *dest, const char *format, ... [const char *source]); + {{"sprintf", 3, 2}, {&StrChecker::evalSprintf, {0}}}, + // int fscanf(FILE *stream, const char *format, ... [char *dest]); + {{"fscanf", 3, 2}, {&StrChecker::evalFscanf, {2}}}, + // char *strcpy(char *dest, const char *src); + {{"strcpy", 2}, {&StrChecker::evalStrcpy, {0, 1}}}}; + + void evalGets(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const; + void evalSprintf(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const; + void evalFscanf(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const; + void evalStrcpy(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const; +}; +} // namespace + +REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(ReportSet, + const PathSensitiveBugReport *) + +// Store the reports for false positive suppression. In case we encounter a +// hand-written null termination of the region every report is invalid on it. +REGISTER_MAP_WITH_PROGRAMSTATE(DestinationMap, const MemRegion *, ReportSet) + +//===----------------------------------------------------------------------===// +// Helper functions. +//===----------------------------------------------------------------------===// + +// Returns a string representation of \p E. +static std::string exprToStr(const Expr *E, CheckerContext &C) { + assert(E); + return Lexer::getSourceText( + CharSourceRange::getTokenRange(E->getSourceRange()), C.getSourceManager(), + C.getLangOpts()); +} + +std::unique_ptr<PathSensitiveBugReport> getReport(BugType &BT, + const CallEvent &Call, + const CallContext &CallC, + CheckerContext &C) { + SmallString<128> Msg; + llvm::raw_svector_ostream Out(Msg); + Out << '\'' << Call.getCalleeIdentifier()->getName() + << "' could write outside of "; + + const Expr *DestExpr = Call.getArgExpr(*CallC.DestinationPos); + static constexpr llvm::StringLiteral ArrayName = "array"; + + auto DRE = declRefExpr().bind(ArrayName); + auto ME = memberExpr().bind(ArrayName); + + auto Matches = + match(expr(anyOf(ME, DRE, hasDescendant(ME), hasDescendant(DRE))), + *DestExpr, C.getASTContext()); + + if (Matches.size() == 1) + Out << '\'' << exprToStr(Matches[0].getNodeAs<Expr>(ArrayName), C) << '\''; + else + Out << "the array"; + + auto Report = std::make_unique<PathSensitiveBugReport>( + BT, Out.str(), C.generateNonFatalErrorNode()); + + // Store the report. + SVal DestV = Call.getArgSVal(*CallC.DestinationPos); + if (DestV.isUnknown() && Matches.size() == 1) + DestV = C.getSVal(Matches[0].getNodeAs<Expr>(ArrayName)); + + const MemRegion *BaseMR = DestV.getAsRegion()->getBaseRegion(); + + ProgramStateRef State = C.getState(); + ReportSet::Factory &F = State->get_context<ReportSet>(); + + const ReportSet *TempSet = State->get<DestinationMap>(BaseMR); + ReportSet Set = TempSet ? *TempSet : F.getEmptySet(); + + Set = F.add(Set, Report.get()); + State = State->set<DestinationMap>(BaseMR, Set); + C.addTransition(State, C.getPredecessor()); + + // Track the allocation. + Report->addVisitor(allocation_state::getMallocBRVisitor(DestV.getAsSymbol())); + return Report; +} + +//===----------------------------------------------------------------------===// +// Evaluating problematic function calls. +//===----------------------------------------------------------------------===// + +void StrChecker::evalGets(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const { + if (EnableStr31cChecker) + C.emitReport(getReport(*BT, Call, CallC, C)); +} + +void StrChecker::evalSprintf(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const { + if (EnableStr31cChecker) + C.emitReport(getReport(*BT, Call, CallC, C)); +} + +void StrChecker::evalFscanf(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const { + if (!EnableStr31cChecker) + return; + + const auto *FormatExpr = + dyn_cast<StringLiteral>(Call.getArgExpr(1)->IgnoreImpCasts()); + if (!FormatExpr) + return; + + // FIXME: Handle multiple buffers. + if (FormatExpr->getString() != "%s") + return; + + C.emitReport(getReport(*BT, Call, CallC, C)); +} + +void StrChecker::evalStrcpy(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const { + if (!EnableStr31cChecker) + return; + + ProgramStateRef State = C.getState(); + SValBuilder &SVB = C.getSValBuilder(); + SVal SrcV = Call.getArgSVal(*CallC.SourcePos); + SVal DestV = Call.getArgSVal(*CallC.DestinationPos); + + // Check the size of the allocation to prevent false alarms. + const MemRegion *SrcMR = SrcV.getAsRegion()->getBaseRegion(); + const MemRegion *DestMR = DestV.getAsRegion()->getBaseRegion(); + DefinedOrUnknownSVal SrcSize = getDynamicSize(State, SrcMR, SVB); + DefinedOrUnknownSVal DestSize = getDynamicSize(State, DestMR, SVB); + + // '13 * strlen(src) + 1' is fine + // FIXME: Use the 'SValVisitor' to catch every such constructs of the symbol. + if (const SymExpr *SE = DestSize.getAsSymExpr()) + for (const auto &Sym : SE->symbols()) + if (const auto *SIE = dyn_cast<SymIntExpr>(Sym)) + if (SIE->getOpcode() == BO_Add) + if (SIE->getRHS().isOneValue()) + if (const auto *SM = dyn_cast<SymbolMetadata>(SIE->getLHS())) + if (SM->getRegion() == SrcMR) + return; + + // 'StringRegion' returns the size with the null-terminator. + if (const llvm::APSInt *SrcSizeInt = SVB.getKnownValue(State, SrcSize)) + if (const llvm::APSInt *DestSizeInt = SVB.getKnownValue(State, DestSize)) + if (SrcSizeInt->getZExtValue() <= DestSizeInt->getZExtValue()) + return; + + C.emitReport(getReport(*BT, Call, CallC, C)); +} + +//===----------------------------------------------------------------------===// +// Main logic to evaluate a call. +//===----------------------------------------------------------------------===// + +bool StrChecker::evalCall(const CallEvent &Call, CheckerContext &C) const { + if (const auto *Lookup = CDM.lookup(Call)) { + const StrCheck &Check = Lookup->first; + Check(this, Call, Lookup->second, C); + } + + return false; +} + +void StrChecker::checkBind(SVal L, SVal V, const Stmt *S, + CheckerContext &C) const { + const auto *BO = dyn_cast<BinaryOperator>(S); + if (!BO) + return; + + const MemRegion *MR = L.getAsRegion(); + if (!MR) + return; + + MR = MR->getBaseRegion(); + + bool IsNullTermination = false; + const Expr *RhsExpr = BO->getRHS()->IgnoreParenImpCasts(); + if (const auto *CL = dyn_cast<CharacterLiteral>(RhsExpr)) { + if (CL->getValue() == 0) + IsNullTermination = true; + } else if (const auto *IL = dyn_cast<IntegerLiteral>(RhsExpr)) { + if (IL->getValue().getZExtValue() == 0) + IsNullTermination = true; + } + + if (!IsNullTermination) + return; + + // Check for wrong assumptions and invalidate them. + const DestinationMapTy &Map = C.getState()->get<DestinationMap>(); + const ReportSet *Set = Map.lookup(MR); + if (!Set) + return; + + BugReporter &Reporter = C.getBugReporter(); + auto EQClasses = + llvm::make_range(Reporter.EQClasses_begin(), Reporter.EQClasses_end()); + + bool IsFalsePositiveFound = false; + for (auto &EQ : EQClasses) { + for (auto &Report : EQ.getReports()) { + auto *PR = dyn_cast<PathSensitiveBugReport>(Report.get()); + if (!PR || !Set->contains(PR)) + continue; + + const ExplodedNode *ErrorNode = PR->getErrorNode(); + const ExplodedNode *PredNode = C.getPredecessor(); + do { + if (PredNode->getState() == ErrorNode->getState()) { + IsFalsePositiveFound = true; + PR->markInvalid(nullptr, nullptr); + break; + } + } while ((PredNode = PredNode->getFirstPred())); + } + } + + if (IsFalsePositiveFound) + C.generateSink(C.getState(), C.getPredecessor()); +} + +void ento::registerStrChecker(CheckerManager &Mgr) { + auto *Checker = Mgr.registerChecker<StrChecker>(); + + Checker->BT = std::make_unique<BugType>( + Mgr.getCurrentCheckerName(), "Insecure string handler function call", + categories::SecurityError); +} + +bool ento::shouldRegisterStrChecker(const LangOptions &) { return true; } + +#define REGISTER_CHECKER(name) \ + void ento::register##name(CheckerManager &mgr) { \ + auto *Checker = mgr.getChecker<StrChecker>(); \ + Checker->Enable##name = true; \ + } \ + \ + bool ento::shouldRegister##name(const LangOptions &LO) { return true; } + +REGISTER_CHECKER(Str31cChecker) Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -3374,6 +3374,10 @@ namespace ento { namespace allocation_state { +std::unique_ptr<BugReporterVisitor> getMallocBRVisitor(SymbolRef Sym) { + return std::make_unique<MallocBugVisitor>(Sym); +} + ProgramStateRef markReleased(ProgramStateRef State, SymbolRef Sym, const Expr *Origin) { AllocationFamily Family = AF_InnerBuffer; Index: clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt =================================================================== --- clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -17,6 +17,7 @@ CastSizeChecker.cpp CastToStructChecker.cpp CastValueChecker.cpp + cert/StrChecker.cpp CheckObjCDealloc.cpp CheckObjCInstMethSignature.cpp CheckSecuritySyntaxOnly.cpp Index: clang/lib/StaticAnalyzer/Checkers/AllocationState.h =================================================================== --- clang/lib/StaticAnalyzer/Checkers/AllocationState.h +++ clang/lib/StaticAnalyzer/Checkers/AllocationState.h @@ -25,6 +25,9 @@ /// AF_InnerBuffer symbols. std::unique_ptr<BugReporterVisitor> getInnerPointerBRVisitor(SymbolRef Sym); +/// \returns The MallocBugVisitor. +std::unique_ptr<BugReporterVisitor> getMallocBRVisitor(SymbolRef Sym); + /// 'Sym' represents a pointer to the inner buffer of a container object. /// This function looks up the memory region of that object in /// DanglingInternalBufferChecker's program state map. Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h @@ -85,6 +85,9 @@ symbol_iterator symbol_begin() const { return symbol_iterator(this); } static symbol_iterator symbol_end() { return symbol_iterator(); } + llvm::iterator_range<symbol_iterator> symbols() const { + return llvm::make_range(symbol_begin(), symbol_end()); + } virtual unsigned computeComplexity() const = 0; Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSizeInfo.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSizeInfo.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSizeInfo.h @@ -9,13 +9,13 @@ #ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_DYNAMICSIZEINFO_H #define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_DYNAMICSIZEINFO_H -namespace clang { -namespace ento { - #include "clang/AST/Expr.h" #include "clang/Basic/LLVM.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h" +namespace clang { +namespace ento { + /// Helper class to store information about the dynamic size. class DynamicSizeInfo { public: Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h +++ clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h @@ -11,16 +11,17 @@ // Common strings used for the "category" of many static analyzer issues. namespace clang { - namespace ento { - namespace categories { - extern const char * const CoreFoundationObjectiveC; - extern const char * const LogicError; - extern const char * const MemoryRefCount; - extern const char * const MemoryError; - extern const char * const UnixAPI; - extern const char * const CXXObjectLifecycle; - } - } -} -#endif +namespace ento { +namespace categories { +extern const char *const CoreFoundationObjectiveC; +extern const char *const LogicError; +extern const char *const MemoryRefCount; +extern const char *const MemoryError; +extern const char *const UnixAPI; +extern const char *const CXXObjectLifecycle; +extern const char *const SecurityError; +} // namespace categories +} // namespace ento +} // namespace clang +#endif // LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_COMMONBUGCATEGORIES_H Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -71,6 +71,9 @@ def SecurityAlpha : Package<"security">, ParentPackage<Alpha>; def Taint : Package<"taint">, ParentPackage<SecurityAlpha>; +def CERT : Package<"cert">, ParentPackage<Security>; +def STR : Package<"str">, ParentPackage<CERT>; + def Unix : Package<"unix">; def UnixAlpha : Package<"unix">, ParentPackage<Alpha>; def CString : Package<"cstring">, ParentPackage<Unix>; @@ -785,6 +788,19 @@ } // end "security" +let ParentPackage = STR in { + +def StrChecker : Checker<"str">, + HelpText<"CERT checker of rules defined in STR.">, + Documentation<NotDocumented>; + +def Str31cChecker : Checker<"31.c">, + HelpText<"CERT checker of rules defined in STR31-C.">, + Dependencies<[StrChecker]>, + Documentation<NotDocumented>; + +} // end "cert.str" + let ParentPackage = SecurityAlpha in { def ArrayBoundChecker : Checker<"ArrayBound">,
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
