Index: clang/docs/analyzer/checkers.rst =================================================================== --- clang/docs/analyzer/checkers.rst +++ clang/docs/analyzer/checkers.rst @@ -2008,7 +2008,17 @@ return putenv(env); // putenv function should not be called with auto variables } - + +.. _alpha-security-cert-str-31c: + +alpha.security.cert.str.31c +""""""""""""""""""""""""""" + +Checker for `STR31-C rule`_. + +It warns on misusing the following functions: +``strcpy()``, ``gets()``, ``fscanf()``, ``sprintf()``. + .. _alpha-security-ArrayBound: alpha.security.ArrayBound (C) Index: clang/include/clang/AST/FormatStringParsing.h =================================================================== --- clang/include/clang/AST/FormatStringParsing.h +++ clang/include/clang/AST/FormatStringParsing.h @@ -90,6 +90,25 @@ const T &getValue() { return FS; } }; +typedef SpecifierResult PrintfSpecifierResult; + +PrintfSpecifierResult ParsePrintfSpecifier(FormatStringHandler &H, + const char *&Beg, const char *E, + unsigned &argIndex, + const LangOptions &LO, + const TargetInfo &Target, bool Warn, + bool isFreeBSDKPrintf); + +typedef SpecifierResult ScanfSpecifierResult; + +// FIXME: Much of this is copy-paste from ParsePrintfSpecifier. +// We can possibly refactor. +ScanfSpecifierResult ParseScanfSpecifier(FormatStringHandler &H, + const char *&Beg, const char *E, + unsigned &argIndex, + const LangOptions &LO, + const TargetInfo &Target); + } // end analyze_format_string namespace } // end clang namespace Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -73,6 +73,7 @@ def CERT : Package<"cert">, ParentPackage; def POS : Package<"pos">, ParentPackage; +def STR : Package<"str">, ParentPackage; def Unix : Package<"unix">; def UnixAlpha : Package<"unix">, ParentPackage; @@ -923,6 +924,20 @@ } // end "alpha.cert.pos" +let ParentPackage = STR in { + +def StrCheckerBase : Checker<"StrCheckerBase">, + HelpText<"SEI CERT base checker of rules defined in STR">, + Documentation, + Hidden; + +def Str31cChecker : Checker<"31c">, + HelpText<"SEI CERT checker of rules defined in STR31-C">, + Dependencies<[StrCheckerBase]>, + Documentation; + +} // end "alpha.cert.str" + let ParentPackage = SecurityAlpha in { def ArrayBoundChecker : Checker<"ArrayBound">, Index: clang/lib/AST/FormatString.cpp =================================================================== --- clang/lib/AST/FormatString.cpp +++ clang/lib/AST/FormatString.cpp @@ -11,7 +11,7 @@ // //===----------------------------------------------------------------------===// -#include "FormatStringParsing.h" +#include "clang/AST/FormatStringParsing.h" #include "clang/Basic/LangOptions.h" #include "clang/Basic/TargetInfo.h" #include "llvm/Support/ConvertUTF.h" Index: clang/lib/AST/PrintfFormatString.cpp =================================================================== --- clang/lib/AST/PrintfFormatString.cpp +++ clang/lib/AST/PrintfFormatString.cpp @@ -11,8 +11,8 @@ // //===----------------------------------------------------------------------===// -#include "FormatStringParsing.h" #include "clang/AST/FormatString.h" +#include "clang/AST/FormatStringParsing.h" #include "clang/AST/OSLog.h" #include "clang/Basic/TargetInfo.h" #include "llvm/Support/Regex.h" @@ -26,9 +26,6 @@ using namespace clang; -typedef clang::analyze_format_string::SpecifierResult - PrintfSpecifierResult; - //===----------------------------------------------------------------------===// // Methods for parsing format strings. //===----------------------------------------------------------------------===// @@ -68,14 +65,13 @@ return true; } -static PrintfSpecifierResult ParsePrintfSpecifier(FormatStringHandler &H, - const char *&Beg, - const char *E, - unsigned &argIndex, - const LangOptions &LO, - const TargetInfo &Target, - bool Warn, - bool isFreeBSDKPrintf) { +analyze_format_string::PrintfSpecifierResult +analyze_format_string::ParsePrintfSpecifier(FormatStringHandler &H, + const char *&Beg, const char *E, + unsigned &argIndex, + const LangOptions &LO, + const TargetInfo &Target, bool Warn, + bool isFreeBSDKPrintf) { using namespace clang::analyze_format_string; using namespace clang::analyze_printf; Index: clang/lib/AST/ScanfFormatString.cpp =================================================================== --- clang/lib/AST/ScanfFormatString.cpp +++ clang/lib/AST/ScanfFormatString.cpp @@ -12,7 +12,7 @@ //===----------------------------------------------------------------------===// #include "clang/AST/FormatString.h" -#include "FormatStringParsing.h" +#include "clang/AST/FormatStringParsing.h" #include "clang/Basic/TargetInfo.h" using clang::analyze_format_string::ArgType; @@ -25,9 +25,6 @@ using clang::UpdateOnReturn; using namespace clang; -typedef clang::analyze_format_string::SpecifierResult - ScanfSpecifierResult; - static bool ParseScanList(FormatStringHandler &H, ScanfConversionSpecifier &CS, const char *&Beg, const char *E) { @@ -72,12 +69,12 @@ // FIXME: Much of this is copy-paste from ParsePrintfSpecifier. // We can possibly refactor. -static ScanfSpecifierResult ParseScanfSpecifier(FormatStringHandler &H, - const char *&Beg, - const char *E, - unsigned &argIndex, - const LangOptions &LO, - const TargetInfo &Target) { +analyze_format_string::ScanfSpecifierResult +analyze_format_string::ParseScanfSpecifier(FormatStringHandler &H, + const char *&Beg, const char *E, + unsigned &argIndex, + const LangOptions &LO, + const TargetInfo &Target) { using namespace clang::analyze_format_string; using namespace clang::analyze_scanf; const char *I = Beg; Index: clang/lib/StaticAnalyzer/Checkers/AllocationState.h =================================================================== --- clang/lib/StaticAnalyzer/Checkers/AllocationState.h +++ clang/lib/StaticAnalyzer/Checkers/AllocationState.h @@ -25,6 +25,8 @@ /// AF_InnerBuffer symbols. std::unique_ptr getInnerPointerBRVisitor(SymbolRef Sym); +std::unique_ptr 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/lib/StaticAnalyzer/Checkers/CMakeLists.txt =================================================================== --- clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -18,6 +18,7 @@ CastSizeChecker.cpp CastToStructChecker.cpp CastValueChecker.cpp + cert/StrChecker.cpp CheckObjCDealloc.cpp CheckObjCInstMethSignature.cpp CheckPlacementNew.cpp Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -3376,6 +3376,10 @@ namespace ento { namespace allocation_state { +std::unique_ptr getMallocBRVisitor(SymbolRef Sym) { + return std::make_unique(Sym); +} + ProgramStateRef markReleased(ProgramStateRef State, SymbolRef Sym, const Expr *Origin) { AllocationFamily Family = AF_InnerBuffer; Index: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp =================================================================== --- /dev/null +++ clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp @@ -0,0 +1,405 @@ +//===- 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/x/ptUxBQ +// +// This checker is a base checker which consist of the following checkers: +// - STR31-C: Guarantee that storage for strings has sufficient space for +// character data and the null terminator: +// - https://wiki.sei.cmu.edu/confluence/x/sNUxBQ +// +//===----------------------------------------------------------------------===// + +#include "AllocationState.h" +#include "clang/AST/FormatString.h" +#include "clang/AST/FormatStringParsing.h" +#include "clang/Basic/TargetInfo.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 + +using namespace clang; +using namespace ento; +using namespace analyze_printf; +using namespace analyze_scanf; +using namespace analyze_format_string; + +namespace { + +/// Helper struct for defining the function call signatures of 'CDM'. +/// The fields are defining what is the index of the interesting arguments. +struct CallContext { + CallContext(Optional DestinationPos, + Optional SourcePos = None) + : DestinationPos(DestinationPos), SourcePos(SourcePos) {} + + Optional DestinationPos; + Optional SourcePos; +}; + +class StrCheckerBase : public Checker { +public: + /// Check the function calls defined in 'CDM'. + /// + /// - In case of STR31-C we want to emit an error report immediately on + /// possible buffer overflows caused by function calls defined in 'CDM' under + /// STR31-C. + void checkPostCall(const CallEvent &Call, CheckerContext &C) const; + + /// \{ + /// Check the function calls defined in 'CDM'. + 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; + /// \} + + bool EnableStr31cChecker = false; + +private: + using StrCheck = std::function; + + const CallDescriptionMap> 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 *src]); + {{"sprintf", None, 2}, {&StrCheckerBase::checkSprintf, {0, 2}}}, + // int fscanf(FILE *stream, const char *format, ... [char *dest]); + {{"fscanf", None, 2}, {&StrCheckerBase::checkFscanf, {2}}}, + // char *strcpy(char *dest, const char *src); + {{"strcpy", 2}, {&StrCheckerBase::checkStrcpy, {0, 1}}}}; + + BugType BT{this, "Insecure string handler function call", + categories::SecurityError}; +}; +} // namespace + +//===----------------------------------------------------------------------===// +// Helper functions. +//===----------------------------------------------------------------------===// + +/// Returns the interesting region of \p V. +static const MemRegion *getRegion(SVal V) { + const MemRegion *MR = V.getAsRegion(); + if (!MR) + return nullptr; + + return MR->getBaseRegion(); +} + +/// Print pretty the expression \p E. +std::string printPretty(const Expr *E, CheckerContext &C) { + assert(E); + SmallString<128> Str; + llvm::raw_svector_ostream Out(Str); + E->printPretty(Out, /*Helper=*/nullptr, + C.getASTContext().getPrintingPolicy()); + return Out.str().str(); +} + +/// It tries to create an error report of possible buffer overflow. +/// +/// \param DestPosOffset In case of a variadic function there are multiple +/// destination arguments so that we need to pass the +/// offset from the first one currently being modeled. +static void createCallOverflowReport(const CallEvent &Call, + const CallContext &CallC, + CheckerContext &C, const BugType &BT, + unsigned DestPosOffset = 0) { + unsigned DestPos = *CallC.DestinationPos + DestPosOffset; + SVal DestV = Call.getArgSVal(DestPos); + // FIXME: Handle offsets. + const MemRegion *MR = getRegion(DestV); + if (!MR) + return; + + const ExplodedNode *ErrorNode = C.generateErrorNode(); + if (!ErrorNode) + 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) + << '\''; + + auto Report = + std::make_unique(BT, Out.str(), ErrorNode); + Report->addRange(Arg->getSourceRange()); + + // Track the allocation. + bugreporter::trackExpressionValue(ErrorNode, Arg, *Report); + if (const SymbolRef Sym = DestV.getAsSymbol()) + Report->addVisitor(allocation_state::getMallocBRVisitor(Sym)); + + C.emitReport(std::move(Report)); +} + +//===----------------------------------------------------------------------===// +// The following checks STR31-C rules. +//===----------------------------------------------------------------------===// + +void StrCheckerBase::checkGets(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const { + if (EnableStr31cChecker) + createCallOverflowReport(Call, CallC, C, BT); +} + +void StrCheckerBase::checkSprintf(const CallEvent &Call, + const CallContext &CallC, + CheckerContext &C) const { + if (!EnableStr31cChecker) + return; + + const auto *FormatExpr = + dyn_cast(Call.getArgExpr(1)->IgnoreImpCasts()); + if (!FormatExpr) + return; + + StringRef FormatStr = FormatExpr->getString(); + unsigned Idx = 0; + const LangOptions &LO = C.getLangOpts(); + const TargetInfo &TI = C.getASTContext().getTargetInfo(); + const char *I = FormatStr.begin(); + const char *E = FormatStr.end(); + FormatStringHandler H; + + ProgramStateRef State = C.getState(); + SValBuilder &SVB = C.getSValBuilder(); + + DefinedOrUnknownSVal DestSize = UnknownVal(); + if (const MemRegion *MR = getRegion(Call.getArgSVal(*CallC.DestinationPos))) + DestSize = getDynamicSize(State, MR, SVB); + + llvm::SmallVector Specifiers; + FormatStr.split(Specifiers, "%"); + + // Iterate over the format string specifiers and add together the required + // sizes into 'RequiredSize'. + SVal RequiredSize = SVB.makeZeroArrayIndex(); + unsigned SrcPosOffset = 0; + while (I != E) { + const PrintfSpecifierResult &R = ParsePrintfSpecifier( + H, I, E, Idx, LO, TI, + /*Warn=*/false, TI.getTriple().getOS() == llvm::Triple::FreeBSD); + if (R.shouldStop()) + break; + + if (!R.hasValue()) + continue; + + // We only need to check string specifiers. + const PrintfSpecifier &PS = R.getValue(); + if (PS.getConversionSpecifier().getKind() != + ConversionSpecifier::Kind::sArg) { + ++SrcPosOffset; + continue; + } + + // Try to obtain the maximal size of the current part of the format string + // or the size of the source argument. Otherwise create an error report. + // FIXME: Handle offsets. + const MemRegion *SrcMR = + getRegion(Call.getArgSVal(*CallC.SourcePos + SrcPosOffset)); + SVal SrcSize = getDynamicSize(State, SrcMR, SVB); + + const OptionalAmount &Size = PS.getPrecision(); + if (Size.getStart()) { + // For example: "%.124s.txt" + // ^~~~ SizeLength + // ^ SizeEnd + // ^~~~ SuffixLength + unsigned TmpRequiredSize = Size.getConstantAmount(); + unsigned SizeLength = Size.getConstantLength(); + const char *SizeEnd = Size.getStart() + SizeLength; + // The first specifier is an empty string due to splitting. + unsigned SuffixLength = Specifiers[SrcPosOffset + 1].end() - SizeEnd - 1; + + TmpRequiredSize += SuffixLength; + RequiredSize = SVB.evalBinOp(State, BO_Add, RequiredSize, + SVB.makeIntVal(TmpRequiredSize, + /*IsUnsigned=*/true), + SVB.getArrayIndexType()); + } else if (SrcSize.getAs()) { + RequiredSize = SVB.evalBinOp(State, BO_Add, RequiredSize, SrcSize, + SVB.getArrayIndexType()); + } else { + // There is no precision set and we cannot measure the size of the source. + createCallOverflowReport(Call, CallC, C, BT); + return; + } + + ++SrcPosOffset; + } + + // We need to be able to store the null-terminator at the end so that + // the enough destination space is RequiredSize < DestSize. + SVal IsEnoughSpace = SVB.evalBinOp(State, BO_LT, RequiredSize, DestSize, + SVB.getArrayIndexType()); + if (State->isNull(IsEnoughSpace).isConstrainedTrue()) + createCallOverflowReport(Call, CallC, C, BT); +} + +void StrCheckerBase::checkFscanf(const CallEvent &Call, + const CallContext &CallC, + CheckerContext &C) const { + if (!EnableStr31cChecker) + return; + + const auto *FormatExpr = + dyn_cast(Call.getArgExpr(1)->IgnoreImpCasts()); + if (!FormatExpr) + return; + + StringRef FormatStr = FormatExpr->getString(); + unsigned Idx = 0; + const LangOptions &LO = C.getLangOpts(); + const TargetInfo &TI = C.getASTContext().getTargetInfo(); + const char *I = FormatStr.begin(); + const char *E = FormatStr.end(); + FormatStringHandler H; + + ProgramStateRef State = C.getState(); + SValBuilder &SVB = C.getSValBuilder(); + + // Iterate over the format string specifiers. + unsigned DestPosOffset = 0; + while (I != E) { + const ScanfSpecifierResult &R = ParseScanfSpecifier(H, I, E, Idx, LO, TI); + if (R.shouldStop()) + break; + + if (!R.hasValue()) + continue; + + // We only need to check string specifiers. + const ScanfSpecifier &SS = R.getValue(); + if (SS.getConversionSpecifier().getKind() != + ConversionSpecifier::Kind::sArg) { + ++DestPosOffset; + continue; + } + + // Try to obtain the maximal size of the current part of the format string. + const OptionalAmount &Size = SS.getFieldWidth(); + if (Size.getStart()) { + // FIXME: Handle offsets. + if (const MemRegion *DestMR = getRegion( + Call.getArgSVal(*CallC.DestinationPos + DestPosOffset))) { + SVal RequiredSize = SVB.makeIntVal(Size.getConstantAmount(), + /*IsUnsigned=*/true); + DefinedOrUnknownSVal DestSize = getDynamicSize(State, DestMR, SVB); + // We need to be able to store the null-terminator at the end so that + // the enough destination space is RequiredSize < DestSize. + SVal IsEnoughSpace = SVB.evalBinOp(State, BO_LT, RequiredSize, DestSize, + SVB.getArrayIndexType()); + if (State->isNull(IsEnoughSpace).isConstrainedTrue()) { + createCallOverflowReport(Call, CallC, C, BT, DestPosOffset); + return; + } + } + } else { + // There is no field width set. + createCallOverflowReport(Call, CallC, C, BT, DestPosOffset); + return; + } + + ++DestPosOffset; + } +} + +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 'DestSize' 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); + + // Only care about allocations we could model. + // FIXME: Use heuristics what is an allocator and try to obtain its size. + if (DestSize.isUnknown()) + return; + + if (SrcSize == DestSize) + return; + + if (const llvm::APSInt *SrcSizeInt = SVB.getKnownValue(State, SrcSize)) + if (const llvm::APSInt *DestSizeInt = SVB.getKnownValue(State, DestSize)) + if (SrcSizeInt->getZExtValue() <= DestSizeInt->getZExtValue()) + 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(SE)) + if (const auto *SM = dyn_cast(SIE->getLHS())) + if (SM->getRegion() == SrcMR) + return; + + createCallOverflowReport(Call, CallC, C, BT); +} + +//===----------------------------------------------------------------------===// +// Main logic to check a function 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); + } +} + +void ento::registerStrCheckerBase(CheckerManager &Mgr) { + Mgr.registerChecker(); +} + +bool ento::shouldRegisterStrCheckerBase(const CheckerManager &) { return true; } + +#define REGISTER_CHECKER(Name) \ + void ento::register##Name(CheckerManager &Mgr) { \ + auto *Checker = Mgr.getChecker(); \ + Checker->Enable##Name = true; \ + } \ + \ + bool ento::shouldRegister##Name(const CheckerManager &) { return true; } + +REGISTER_CHECKER(Str31cChecker) 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 @@ -32,6 +32,12 @@ int printf(const char *restrict format, ...); int fprintf(FILE *restrict, const char *restrict, ...); int getchar(void); +char *gets(char *buffer); +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 fscanf(FILE *stream, const char *format, ...); +char *getenv(const char *varname); void setbuf(FILE *restrict, char *restrict); int setvbuf(FILE *restrict, char *restrict, int, size_t); Index: clang/test/Analysis/cert/str31-c-extras.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/cert/str31-c-extras.cpp @@ -0,0 +1,95 @@ +// RUN: %clang_analyze_cc1 \ +// RUN: -analyzer-checker=core,unix,alpha.security.cert.str.31c \ +// RUN: -verify %s + +// See the examples on the page of STR31-C: +// https://wiki.sei.cmu.edu/confluence/x/sNUxBQ + +#include "../Inputs/system-header-simulator.h" + +void free(void *memblock); +void *malloc(size_t size); + +// FIXME: The following tests could fail if the destination array has an offset, +// which we do not model yet. Also we cannot reason about complex sized +// allocations which makes this checker a little bit noisy. + +void test_known_size_cannot_overflow() { + char dest[128]; + strcpy(dest, "src"); // no-warning +} + +void test_everything_known() { + char *dest = (char *)malloc(strlen("Foo") + 1); + if (dest) + strcpy(dest, "Foo"); // no-warning + free(dest); +} + +void test_complex_size_may_false_positive(const char *src) { + char *dest; + size_t size; + size = strlen(src); + + dest = (char *)malloc(size * 2 + 2); + + strcpy(dest, &src[13]); + // expected-warning@-1 {{'strcpy' could write outside of 'dest'}} + free(dest); +} + +void test_complex_size_wrong_size(const char *src) { + char *dest; + size_t size = strlen(src); + + // FIXME: '+ 2' missing so there is not enough space for the null terminator. + dest = (char *)malloc(2 * size + 1); + + strcpy(dest, &src[13]); + // expected-warning@-1 {{'strcpy' could write outside of 'dest'}} + free(dest); +} + +void test_char_by_char_may_false_positive(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"); + // expected-warning@-1 {{'strcpy' could write outside of 'e'}} + e += 2; + } else { + *e++ = c; + } + } + + *e = '\0'; + free(escape); +} + +void test_format_string_read_max_value_single() { + char buf[1]; + fscanf(stdin, "%2s", buf); + // expected-warning@-1 {{'fscanf' could write outside of 'buf'}} +} + +void test_format_string_read_max_value_multiple_not_string_specifier() { + char d[3], y[4]; + unsigned m; + fscanf(stdin, "%2s;%u;%4s;", d, &m, y); + // expected-warning@-1 {{'fscanf' could write outside of 'y'}} +} + +void test_format_string_write_max_value_single(const char *name) { + char filename[128]; // 124 + '.txt' + '\0' = 129 characters + sprintf(filename, "%.124s.txt", name); + // expected-warning@-1 {{'sprintf' could write outside of 'filename'}} +} + +void test_format_string_write_max_value_multiple_known_src(const char *name) { + char filename[128]; + sprintf(filename, "%.123s.%s", name, "extension"); + // expected-warning@-1 {{'sprintf' could write outside of 'filename'}} +} Index: clang/test/Analysis/cert/str31-c-notes.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/cert/str31-c-notes.cpp @@ -0,0 +1,47 @@ +// RUN: %clang_analyze_cc1 \ +// RUN: -analyzer-checker=core,unix,alpha.security.cert.str.31c \ +// RUN: -analyzer-output=text -verify %s + +// See the examples on the page of STR31-C: +// https://wiki.sei.cmu.edu/confluence/x/sNUxBQ + +#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}} + // expected-note@-2 {{'buf' initialized here}} + + if (gets(buf)) { + // expected-note@-1 {{'gets' could write outside of 'buf'}} + // expected-warning@-2 {{'gets' could write outside of 'buf'}} + } + + free(buf); +} + +void test_strlen_without_null_terminator(const char *src) { + char *buff2; + char *editor = getenv("EDITOR"); + if (editor != NULL) { + // expected-note@-1 {{Assuming 'editor' is not equal to NULL}} + // expected-note@-2 {{Taking true branch}} + size_t len = strlen(editor); + buff2 = (char *)malloc(len); + // expected-note@-1 {{Memory is allocated}} + // expected-note@-2 {{Value assigned to 'buff2'}} + if (buff2 != NULL) { + // expected-note@-1 {{Assuming 'buff2' is not equal to NULL}} + // expected-note@-2 {{Taking true branch}} + strcpy(buff2, editor); + // expected-note@-1 {{'strcpy' could write outside of 'buff2'}} + // expected-warning@-2 {{'strcpy' could write outside of 'buff2'}} + } + free(buff2); + } +} Index: clang/test/Analysis/cert/str31-c.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/cert/str31-c.cpp @@ -0,0 +1,184 @@ +// RUN: %clang_analyze_cc1 \ +// RUN: -analyzer-checker=core,unix,alpha.security.cert.str.31c \ +// RUN: -verify %s + +// See the examples on the page of STR31-C: +// https://wiki.sei.cmu.edu/confluence/x/sNUxBQ + +#include "../Inputs/system-header-simulator.h" + +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]; + 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]; + 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 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