Index: clang/docs/analyzer/checkers.rst =================================================================== --- clang/docs/analyzer/checkers.rst +++ clang/docs/analyzer/checkers.rst @@ -1890,6 +1890,38 @@ alpha.security ^^^^^^^^^^^^^^ + + +alpha.security.cert +^^^^^^^^^^^^^^^^^^^ + +SEI CERT checkers which tries to find errors based on their `C coding rules`_. + +.. _alpha-security-cert-pos-checkers: + +alpha.security.cert.pos +^^^^^^^^^^^^^^^^^^^^^^^ + +SEI CERT checkers of POSIX `C coding rules`_. + +.. _alpha-security-cert-pos-34c: + +alpha.security.cert.pos.34c +""""""""""""""""""""""""""" +Finds calls to the ``putenv`` function which pass a pointer to an automatic variable as the argument. + +.. code-block:: c + + int func(const char *var) { + char env[1024]; + int retval = snprintf(env, sizeof(env),"TEST=%s", var); + if (retval < 0 || (size_t)retval >= sizeof(env)) { + /* Handle error */ + } + + return putenv(env); // putenv function should not be called with auto variables + } + .. _alpha-security-ArrayBound: alpha.security.ArrayBound (C) Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -71,6 +71,9 @@ def SecurityAlpha : Package<"security">, ParentPackage; def Taint : Package<"taint">, ParentPackage; +def CERT : Package<"cert">, ParentPackage; +def POS : Package<"pos">, ParentPackage; + def Unix : Package<"unix">; def UnixAlpha : Package<"unix">, ParentPackage; def CString : Package<"cstring">, ParentPackage; @@ -785,6 +788,15 @@ } // end "security" +let ParentPackage = POS in { + + def PutenvWithAuto : Checker<"34c">, + HelpText<"Finds calls to the `putenv` function which pass a pointer to " + "an automatic variable as the argument. (CERT POS 34C)">, + Documentation; + +} // end "alpha.cert.pos" + let ParentPackage = SecurityAlpha in { def ArrayBoundChecker : Checker<"ArrayBound">, 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,16 @@ // 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; - } - } -} +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 - 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 getInnerPointerBRVisitor(SymbolRef Sym); +/// \returns The MallocBugVisitor. +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 @@ -85,6 +85,7 @@ PointerSortingChecker.cpp PointerSubChecker.cpp PthreadLockChecker.cpp + cert/PutenvWithAutoChecker.cpp RetainCountChecker/RetainCountChecker.cpp RetainCountChecker/RetainCountDiagnostics.cpp ReturnPointerRangeChecker.cpp Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -3380,6 +3380,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/PutenvWithAutoChecker.cpp =================================================================== --- /dev/null +++ clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp @@ -0,0 +1,75 @@ +//== PutenvWithAutoChecker.cpp --------------------------------- -*- 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 PutenvWithAutoChecker which finds calls of ``putenv`` +// function with automatic variable as the argument. +// https://wiki.sei.cmu.edu/confluence/display/c/POS34-C.+Do+not+call+putenv%28%29+with+a+pointer+to+an+automatic+variable+as+the+argument +// +//===----------------------------------------------------------------------===// + +#include "AllocationState.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/MemRegion.h" + +using namespace clang; +using namespace ento; + +class PutenvWithAutoChecker : public Checker { +private: + BugType BT{this, "'putenv' function should not be called with auto variables", + categories::SecurityError}; + +public: + void checkPostCall(const CallEvent &Call, CheckerContext &C) const; +}; + +void PutenvWithAutoChecker::checkPostCall(const CallEvent &Call, + CheckerContext &C) const { + if (const IdentifierInfo *II = Call.getCalleeIdentifier()) + if (!II->isStr("putenv")) + return; + + bool IsPossiblyAutoVar = false; + SVal ArgV = Call.getArgSVal(0); + const Expr *ArgExpr = Call.getArgExpr(0); + const MemSpaceRegion *MSR = ArgV.getAsRegion()->getMemorySpace(); + + if (const auto *DRE = dyn_cast(ArgExpr->IgnoreImpCasts())) + if (const auto *VD = dyn_cast(DRE->getDecl())) + IsPossiblyAutoVar = isa(VD) && isa(MSR); + + if (!IsPossiblyAutoVar && + (!isa(MSR) && !isa(MSR))) + return; + + StringRef ErrorMsg = + "'putenv' function should not be called with auto variables"; + ExplodedNode *N = C.generateErrorNode(); + auto Report = std::make_unique(BT, ErrorMsg, N); + + // Track the argument. + if (isa(MSR)) { + bugreporter::trackExpressionValue(Report->getErrorNode(), ArgExpr, *Report); + } else if (const SymbolRef Sym = + ArgV.getAsSymbol()) { // It is a `HeapSpaceRegion` + Report->addVisitor(allocation_state::getMallocBRVisitor(Sym)); + } + + C.emitReport(std::move(Report)); +} + +void ento::registerPutenvWithAuto(CheckerManager &Mgr) { + Mgr.registerChecker(); +} + +bool ento::shouldRegisterPutenvWithAuto(const LangOptions &) { return true; } Index: clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp +++ clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp @@ -9,13 +9,18 @@ #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 = "Security error"; +} // namespace categories +} // namespace ento +} // namespace clang Index: clang/test/Analysis/cert/pos34-c-fp-suppression.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/cert/pos34-c-fp-suppression.cpp @@ -0,0 +1,69 @@ +// RUN: %clang_analyze_cc1 \ +// RUN: -analyzer-checker=alpha.security.cert.pos.34c\ +// RUN: -verify %s + +#include "../Inputs/system-header-simulator.h" +void free(void *memblock); +void *malloc(size_t size); +int putenv(char *); +int snprintf(char *str, size_t size, const char *format, ...); +int rand(); + +namespace test_auto_var_used_bad { + +int volatile_memory1(char *a) { + return putenv(a); + // expected-warning@-1 {{'putenv' function should not be called with auto variables}} +} + +void volatile_memory2(char *a) { + char *buff = (char *)"hello"; + putenv(buff); + // expected-warning@-1 {{'putenv' function should not be called with auto variables}} +} + +void foo(void) { + char *buffer = (char *)"huttah!"; + if (rand() % 2 == 0) { + buffer = (char *)malloc(5); + strcpy(buffer, "woot"); + } + putenv(buffer); + // expected-warning@-1 {{'putenv' function should not be called with auto variables}} +} + +void bar(void) { + char *buffer = (char *)malloc(5); + strcpy(buffer, "woot"); + + if (rand() % 2 == 0) { + free(buffer); + buffer = (char *)"blah blah blah"; + } + putenv(buffer); + // expected-warning@-1 {{'putenv' function should not be called with auto variables}} +} + +void baz() { + char env[] = "NAME=value"; + // TODO: False Positive + putenv(env); + // expected-warning@-1 {{'putenv' function should not be called with auto variables}} + + /* + DO SOMETHING + */ + + putenv((char *)"NAME=anothervalue"); +} + +} // namespace test_auto_var_used_bad + +namespace test_auto_var_used_good { + +extern char *ex; +int test_extern() { + return putenv(ex); // no-warning: extern storage class. +} + +} // namespace test_auto_var_used_good Index: clang/test/Analysis/cert/pos34-c.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/cert/pos34-c.cpp @@ -0,0 +1,61 @@ +// RUN: %clang_analyze_cc1 \ +// RUN: -analyzer-checker=alpha.security.cert.pos.34c\ +// RUN: -verify %s + +// Examples from the CERT rule's page. +// https://wiki.sei.cmu.edu/confluence/display/c/POS34-C.+Do+not+call+putenv%28%29+with+a+pointer+to+an+automatic+variable+as+the+argument + +#include "../Inputs/system-header-simulator.h" +void free(void *memblock); +void *malloc(size_t size); +int putenv(char *); +int snprintf(char *str, size_t size, const char *format, ...); + +namespace test_auto_var_used_bad { + +int volatile_memory1(const char *var) { + char env[1024]; + int retval = snprintf(env, sizeof(env), "TEST=%s", var); + if (retval < 0 || (size_t)retval >= sizeof(env)) { + /* Handle error */ + } + + return putenv(env); + // expected-warning@-1 {{'putenv' function should not be called with auto variables}} +} + +} // namespace test_auto_var_used_bad + +namespace test_auto_var_used_good { + +int test_static(const char *var) { + static char env[1024]; + + int retval = snprintf(env, sizeof(env), "TEST=%s", var); + if (retval < 0 || (size_t)retval >= sizeof(env)) { + /* Handle error */ + } + + return putenv(env); +} + +int test_heap_memory(const char *var) { + static char *oldenv; + const char *env_format = "TEST=%s"; + const size_t len = strlen(var) + strlen(env_format); + char *env = (char *)malloc(len); + if (env == NULL) { + return -1; + } + if (putenv(env) != 0) { // no-warning: env was dynamically allocated. + free(env); + return -1; + } + if (oldenv != NULL) { + free(oldenv); /* avoid memory leak */ + } + oldenv = env; + return 0; +} + +} // namespace test_auto_var_used_good