diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -1929,6 +1929,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) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/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; @@ -829,6 +832,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.">, + Documentation; + +} // end "alpha.cert.pos" + let ParentPackage = SecurityAlpha in { def ArrayBoundChecker : Checker<"ArrayBound">, diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h --- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h +++ b/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 - diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -89,6 +89,7 @@ PointerSortingChecker.cpp PointerSubChecker.cpp PthreadLockChecker.cpp + cert/PutenvWithAutoChecker.cpp RetainCountChecker/RetainCountChecker.cpp RetainCountChecker/RetainCountDiagnostics.cpp ReturnPointerRangeChecker.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp new file mode 100644 --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp @@ -0,0 +1,64 @@ +//== 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/x/6NYxBQ +// +//===----------------------------------------------------------------------===// + +#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}; + const CallDescription Putenv{"putenv", 1}; + +public: + void checkPostCall(const CallEvent &Call, CheckerContext &C) const; +}; + +void PutenvWithAutoChecker::checkPostCall(const CallEvent &Call, + CheckerContext &C) const { + if (!Call.isCalled(Putenv)) + return; + + SVal ArgV = Call.getArgSVal(0); + const Expr *ArgExpr = Call.getArgExpr(0); + const MemSpaceRegion *MSR = ArgV.getAsRegion()->getMemorySpace(); + + if (!isa(MSR)) + return; + + StringRef ErrorMsg = "The 'putenv' function should not be called with " + "arguments that have automatic storage"; + ExplodedNode *N = C.generateErrorNode(); + auto Report = std::make_unique(BT, ErrorMsg, N); + + // Track the argument. + bugreporter::trackExpressionValue(Report->getErrorNode(), ArgExpr, *Report); + + C.emitReport(std::move(Report)); +} + +void ento::registerPutenvWithAuto(CheckerManager &Mgr) { + Mgr.registerChecker(); +} + +bool ento::shouldRegisterPutenvWithAuto(const LangOptions &) { return true; } diff --git a/clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp b/clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp --- a/clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp +++ b/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 diff --git a/clang/test/Analysis/cert/pos34-c-fp-suppression.cpp b/clang/test/Analysis/cert/pos34-c-fp-suppression.cpp new file mode 100644 --- /dev/null +++ b/clang/test/Analysis/cert/pos34-c-fp-suppression.cpp @@ -0,0 +1,51 @@ +// 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 rand(); + +namespace test_auto_var_used_good { + +extern char *ex; +int test_extern() { + return putenv(ex); // no-warning: extern storage class. +} + +void foo(void) { + char *buffer = (char *)"huttah!"; + if (rand() % 2 == 0) { + buffer = (char *)malloc(5); + strcpy(buffer, "woot"); + } + putenv(buffer); +} + +void bar(void) { + char *buffer = (char *)malloc(5); + strcpy(buffer, "woot"); + + if (rand() % 2 == 0) { + free(buffer); + buffer = (char *)"blah blah blah"; + } + putenv(buffer); +} + +void baz() { + char env[] = "NAME=value"; + // TODO: False Positive + putenv(env); + // expected-warning@-1 {{The 'putenv' function should not be called with arguments that have automatic storage}} + + /* + DO SOMETHING + */ + + putenv((char *)"NAME=anothervalue"); +} + +} // namespace test_auto_var_used_good diff --git a/clang/test/Analysis/cert/pos34-c.cpp b/clang/test/Analysis/cert/pos34-c.cpp new file mode 100644 --- /dev/null +++ b/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/x/6NYxBQ + +#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 {{The 'putenv' function should not be called with arguments that have automatic storage}} +} + +} // 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