Charusso updated this revision to Diff 232198. Charusso added a comment. - Tweaking.
CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70411/new/ 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/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,205 @@ +// 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-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 "../Inputs/system-header-simulator.h" + +#define EOF -1 +typedef __SIZE_TYPE__ size_t; + +void free(void *memblock); +void *malloc(size_t size); + +void do_something(char *buffer); + +namespace test_gets_bad { +#define BUFFER_SIZE 1024 + +void func(void) { + char buf[BUFFER_SIZE]; + if (gets(buf)) { + do_something(buf); + // expected-warning@-1 {{'buf' is not null-terminated}} + } +} +} // namespace test_gets_bad + +namespace test_gets_good { +enum { BUFFERSIZE = 32 }; + +void func(void) { + char buff[BUFFERSIZE]; + + if (fgets(buff, sizeof(buff), stdin)) { + do_something(buff); + } +} +} // namespace test_gets_good + +namespace test_sprintf_bad { +void func(const char *name) { + char buf[128]; + sprintf(buf, "%s.txt", name); + + do_something(buf); + // expected-warning@-1 {{'buf' is not null-terminated}} +} +} // namespace test_sprintf_bad + +namespace test_sprintf_good { +void func(const char *name) { + char buff[128]; + snprintf(buff, sizeof(buff), "%s.txt", name); + + do_something(buff); +} +} // 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)) { + do_something(buf); + // expected-warning@-1 {{'buf' is not null-terminated}} + } +} +} // 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)) { + do_something(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); + + do_something(prog_name); + // expected-warning@-1 {{'prog_name' is not null-terminated}} + + return 0; +} + +void func(void) { + char buff[256]; + char *editor = getenv("EDITOR"); + if (editor != NULL) { + strcpy(buff, editor); + + do_something(buff); + // expected-warning@-1 {{'buff' is not null-terminated}} + } +} +} // 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); + } + + do_something(prog_name2); + + 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); + } + + do_something(buff2); + free(buff2); + } +} +} // namespace test_strcpy_good + +//===----------------------------------------------------------------------===// +// The following are 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,67 @@ +// 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-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 "../Inputs/system-header-simulator.h" + +void *malloc(size_t size); +void free(void *memblock); + +void do_something(char *buffer); + +void test_simple_size(unsigned size) { + size = 13; + + char *buf = (char *)malloc(size); + // expected-note@-1 {{Memory is allocated}} + + if (gets(buf)) {} + // expected-note@-1 {{'gets' could write outside of 'buf'}} + // expected-note@-2 {{Assuming the condition is false}} + // expected-note@-3 {{Taking false branch}} + + free(buf); + // expected-warning@-1 {{'buf' is not null-terminated}} + // expected-note@-2 {{'buf' is not null-terminated}} +} + +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-note@-1 {{'gets' could write outside of 'buf'}} + // expected-note@-2 {{Assuming the condition is false}} + // expected-note@-3 {{Taking false branch}} + + char *p = buf; + free(p); + // expected-warning@-1 {{'p' is not null-terminated}} + // expected-note@-2 {{'p' is not null-terminated}} +} + +void test_using_before_terminating(const char *src) { + char dest[128]; + // expected-note@-1 {{'dest' initialized here}} + + strcpy(dest, src); + // expected-note@-1 {{'strcpy' could write outside of 'dest'}} + *dest = '\0'; + + strcpy(dest, src); + // expected-note@-1 {{'strcpy' could write outside of 'dest'}} + + do_something(dest); + // expected-warning@-1 {{'dest' is not null-terminated}} + // expected-note@-2 {{'dest' is not null-terminated}} + *dest = '\0'; + + do_something(dest); +} Index: clang/test/Analysis/cert/str31-c-fp-suppression.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/cert/str31-c-fp-suppression.cpp @@ -0,0 +1,116 @@ +// 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-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 "../Inputs/system-header-simulator.h" + +#define EOF -1 + +void free(void *memblock); +void *malloc(size_t size); + +void do_something(char *destfer); + +void test_wonky_null_termination(const char *src) { + char dest[128]; + strcpy(dest, src); + + // For now assume that the explicit null-termination on every path is fine. + dest[sizeof(dest) - 1] = '\0'; + + do_something(dest); // no-warning +} + +void test_may_not_every_path_null_terminated(const char *src) { + char dest[128]; + strcpy(dest, src); + + // FIXME: We need to model more about the dynamic element count of the + // string, so that we could see whether the string is null-terminated on + // every path or not. Because the 'src' could be not null-terminated this + // report is fine, but if 'src' is null-terminated this report is a false + // positive because the 'src' fits into 'dest' with the null terminator + // on the false-branch. + if (strlen(src) > sizeof(dest)) { + dest[sizeof(dest) - 1] = '\0'; + } + + do_something(dest); + // expected-warning@-1 {{'dest' is not null-terminated}} +} + +void test_using_before_terminating(const char *src) { + char dest[128]; + strcpy(dest, src); + *dest = '\0'; + do_something(dest); // no-warning + + strcpy(dest, src); + do_something(dest); + // expected-warning@-1 {{'dest' is not null-terminated}} + + *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"); + do_something(dest); // no-warning +} + +void test_everything_known() { + char *dest = (char *)malloc(strlen("Foo") + 1); + if (dest) + strcpy(dest, "Foo"); + + free(dest); // no-warning +} + +// FIXME: Suppress that with 'SValVisitor' or something similar. +void test_complex_size_false_positive(const char *src) { + char *dest; + size_t size; + size = strlen(src); + + dest = (char *)malloc(size * 2 + 2); + + strcpy(dest, &src[13]); + free(dest); + // expected-warning@-1 {{'dest' is not null-terminated}} +} + +void test_complex_size_wrong_size(const char *src) { + char *dest; + size_t size; + size = strlen(src); + +// FIXME: '+ 2' is missing so there is not enough space for the null terminator. + dest = (char *)malloc(size * 2); + + strcpy(dest, &src[13]); + free(dest); + // expected-warning@-1 {{'dest' is not null-terminated}} +} + +void test_char_by_char(const char *src, size_t size) { + const char *s; + unsigned char c; + char *escape = (char *)malloc(size); + char *e = escape; + for (s = src; (c = *s) != '\0'; ++s) { + if (c == '\n') { + strcpy(e, "\\n"); + e += 2; + } else { + *e++ = c; + } + } + + *e = '\0'; + free(escape); // no-warning +} 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/Checkers/cert/StrChecker.cpp =================================================================== --- /dev/null +++ clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp @@ -0,0 +1,332 @@ +//===- StrCheckerBase - SEI CERT rules checker 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 StrCheckerBase 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 StrCheckerBase : public Checker<check::PostCall, check::Bind> { + using StrCheck = std::function<void(const StrCheckerBase *, const CallEvent &, + const CallContext &, CheckerContext &)>; + +public: + // We report a note when any of the calls in 'CDM' is being used because + // they can cause a not null-terminated string. In this case we store the + // string is being not null-terminated in the 'NullTerminationMap'. + // + // We report an error when the not null-terminated string is passed to a + // function call. Assume that the non-'CDM' calls could read the string. + void checkPostCall(const CallEvent &Call, CheckerContext &C) const; + + // We want to catch the hand-written null-termination of strings to prevent + // false positives. 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; + + void checkGets(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const; + void checkSprintf(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const; + void checkFscanf(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const; + void checkStrcpy(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const; + + std::unique_ptr<BugType> BT; + bool EnableStr31cChecker = false; + +private: + const CallDescriptionMap<std::pair<StrCheck, CallContext>> CDM = { + // The following checks STR31-C rules. + // char *gets(char *dest); + {{"gets", 1}, {&StrCheckerBase::checkGets, {0}}}, + // int sprintf(char *dest, const char *format, ... [const char *source]); + {{"sprintf", 3, 2}, {&StrCheckerBase::checkSprintf, {0}}}, + // 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}}}}; +}; +} // namespace + +// It stores whether the string is null-terminated. +REGISTER_MAP_WITH_PROGRAMSTATE(NullTerminationMap, const MemRegion *, bool) + +//===----------------------------------------------------------------------===// +// 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()); +} + +// Returns the appropriate region of \p V. +static const MemRegion *getRegion(SVal V) { + const MemRegion *MR = V.getAsRegion(); + if (!MR) + return nullptr; + + return MR->getBaseRegion(); +} + +// Tries to obtain a pretty-printable name of \p E. +Optional<std::string> getName(const Expr *E, CheckerContext &C) { + if (!E) + return None; + + static constexpr llvm::StringLiteral Name = "name"; + + auto DRE = declRefExpr().bind(Name); + auto ME = memberExpr().bind(Name); + + auto Matches = + match(expr(anyOf(ME, DRE, hasDescendant(ME), hasDescendant(DRE))), *E, + C.getASTContext()); + + if (Matches.size() == 1) + return '\'' + exprToStr(Matches[0].getNodeAs<Expr>(Name), C) + '\''; + + return None; +} + +static void createOverflowNote(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) { + unsigned DestPos = *CallC.DestinationPos; + SVal DestV = Call.getArgSVal(DestPos); + const MemRegion *MR = getRegion(DestV); + if (!MR) + return; + + ProgramStateRef State = C.getState(); + + std::string CallName = Call.getCalleeIdentifier()->getName(); + Optional<std::string> ArrayName = getName(Call.getArgExpr(DestPos), C); + + const NoteTag *Tag = C.getNoteTag( + [=]() -> std::string { + SmallString<128> Msg; + llvm::raw_svector_ostream Out(Msg); + Out << '\'' << CallName << "' could write outside of " + << (ArrayName ? *ArrayName : "the array"); + + return Out.str(); + }, + /*IsPrunable=*/false); + + // Mark the string could be not null-terminated. + State = State->set<NullTerminationMap>(MR, false); + C.addTransition(State, C.getPredecessor(), Tag); +} + +//===----------------------------------------------------------------------===// +// Evaluating problematic function calls. +//===----------------------------------------------------------------------===// + +void StrCheckerBase::checkGets(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const { + if (EnableStr31cChecker) + createOverflowNote(Call, CallC, C); +} + +void StrCheckerBase::checkSprintf(const CallEvent &Call, + const CallContext &CallC, + CheckerContext &C) const { + if (EnableStr31cChecker) + createOverflowNote(Call, CallC, C); +} + +void StrCheckerBase::checkFscanf(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; + + createOverflowNote(Call, CallC, C); +} + +void StrCheckerBase::checkStrcpy(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 = getRegion(SrcV); + const MemRegion *DestMR = getRegion(DestV); + if (!SrcMR || !DestMR) + return; + + DefinedOrUnknownSVal SrcSize = getDynamicSize(State, SrcMR, SVB); + DefinedOrUnknownSVal DestSize = getDynamicSize(State, DestMR, SVB); + + // 'strlen(src) + integer' is most likely fine. + // FIXME: Use the 'SValVisitor' to catch every such constructs of the symbol. + // FIXME: We cannot catch every '+ integer' part at the moment so we do not + // check that property for now. + 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 (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; + + createOverflowNote(Call, CallC, C); +} + +//===----------------------------------------------------------------------===// +// Main logic to check a call. +//===----------------------------------------------------------------------===// + +void StrCheckerBase::checkPostCall(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; + } + + // If we do not model the call and the given argument is a string which is + // not null-terminated emit a report. + for (unsigned I = 0; I < Call.getNumArgs(); ++I) { + SVal V = Call.getArgSVal(I); + const MemRegion *MR = getRegion(V); + if (!MR) + continue; + + ProgramStateRef State = C.getState(); + const bool *IsNullTerminated = State->get<NullTerminationMap>(MR); + if (!IsNullTerminated || *IsNullTerminated) + continue; + + const Expr *Arg = Call.getArgExpr(I); + SmallString<128> Msg; + llvm::raw_svector_ostream Out(Msg); + + if (Optional<std::string> Name = getName(Arg, C)) + Out << *Name; + else + Out << "the argument"; + Out << " is not null-terminated"; + + auto Report = std::make_unique<PathSensitiveBugReport>( + *BT, Out.str(), C.generateErrorNode()); + Report->addRange(Arg->getSourceRange()); + + // Track the allocation. + const auto *VR = MR->getAs<VarRegion>(); + if (VR && VR->getValueType()->getAsArrayTypeUnsafe()) { + bugreporter::trackExpressionValue(C.getPredecessor(), Arg, *Report); + } else if (const SymbolRef Sym = Call.getArgSVal(I).getAsSymbol()) { + Report->addVisitor(allocation_state::getMallocBRVisitor(Sym)); + } + + C.emitReport(std::move(Report)); + } +} + +void StrCheckerBase::checkBind(SVal L, SVal V, const Stmt *S, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + + // Check for a hand-written null-termination, e.g: 'c_string[length] = '\0';'. + const MemRegion *MR = getRegion(L); + if (!MR) + return; + + bool IsNullTermination = false; + if (const llvm::APInt *Int = C.getSValBuilder().getKnownValue(State, V)) + IsNullTermination = Int->isNullValue(); + + if (!IsNullTermination) + return; + + // Mark the string null-terminated. + State = State->set<NullTerminationMap>(MR, true); + C.addTransition(State, C.getPredecessor()); +} + +void ento::registerStrCheckerBase(CheckerManager &Mgr) { + auto *Checker = Mgr.registerChecker<StrCheckerBase>(); + + Checker->BT = std::make_unique<BugType>( + Mgr.getCurrentCheckerName(), "Insecure string handler function call", + categories::SecurityError); +} + +bool ento::shouldRegisterStrCheckerBase(const LangOptions &) { return true; } + +#define REGISTER_CHECKER(Name) \ + void ento::register##Name(CheckerManager &Mgr) { \ + auto *Checker = Mgr.getChecker<StrCheckerBase>(); \ + 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,20 @@ } // end "security" +let ParentPackage = STR in { + +def StrCheckerBase : Checker<"StrCheckerBase">, + HelpText<"SEI CERT base checker of rules defined in STR">, + Documentation<HasDocumentation>, + Hidden; + +def Str31cChecker : Checker<"31.c">, + HelpText<"SEI CERT checker of rules defined in STR31-C">, + Dependencies<[StrCheckerBase]>, + Documentation<HasDocumentation>; + +} // 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
