[clang] a54d81f - [analyzer] CERT: POS34-C
Zurab Tsinadze via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 19 09:13:52 PST 2020
Author: Zurab Tsinadze
Date: 2020-02-19T18:12:19+01:00
New Revision: a54d81f597963b8768ce2b94a8ef570f9eaaac25
URL: https://github.com/llvm/llvm-project/commit/a54d81f597963b8768ce2b94a8ef570f9eaaac25
DIFF: https://github.com/llvm/llvm-project/commit/a54d81f597963b8768ce2b94a8ef570f9eaaac25.diff
LOG: [analyzer] CERT: POS34-C
Summary:
This patch introduces a new checker:
`alpha.security.cert.pos.34c`
This checker is implemented based on the following rule:
https://wiki.sei.cmu.edu/confluence/x/6NYxBQ
The check warns if `putenv` function is
called with automatic storage variable as an argument.
Differential Revision: https://reviews.llvm.org/D71433
Added:
clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp
clang/test/Analysis/cert/pos34-c-fp-suppression.cpp
clang/test/Analysis/cert/pos34-c.cpp
Modified:
clang/docs/analyzer/checkers.rst
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
Removed:
################################################################################
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index f39bf1f7b2b8..b130cc34f685 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1929,6 +1929,38 @@ Warns against using one vs. many plural pattern in code when generating localize
alpha.security
^^^^^^^^^^^^^^
+
+
+alpha.security.cert
+^^^^^^^^^^^^^^^^^^^
+
+SEI CERT checkers which tries to find errors based on their `C coding rules<https://wiki.sei.cmu.edu/confluence/display/c/2+Rules>`_.
+
+.. _alpha-security-cert-pos-checkers:
+
+alpha.security.cert.pos
+^^^^^^^^^^^^^^^^^^^^^^^
+
+SEI CERT checkers of POSIX `C coding rules<https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152405>`_.
+
+.. _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
index 76cf92af9113..91c0fc88f6e7 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -71,6 +71,9 @@ def InsecureAPI : Package<"insecureAPI">, ParentPackage<Security>;
def SecurityAlpha : Package<"security">, ParentPackage<Alpha>;
def Taint : Package<"taint">, ParentPackage<SecurityAlpha>;
+def CERT : Package<"cert">, ParentPackage<SecurityAlpha>;
+def POS : Package<"pos">, ParentPackage<CERT>;
+
def Unix : Package<"unix">;
def UnixAlpha : Package<"unix">, ParentPackage<Alpha>;
def CString : Package<"cstring">, ParentPackage<Unix>;
@@ -829,6 +832,15 @@ def FloatLoopCounter : Checker<"FloatLoopCounter">,
} // 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<HasDocumentation>;
+
+} // 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
index 22c1a7dd98cc..637b89fd9036 100644
--- 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
index 765c2f3e4530..b7fb0d90c980 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -89,6 +89,7 @@ add_clang_library(clangStaticAnalyzerCheckers
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
index 000000000000..c3295d6b16cb
--- /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<check::PostCall> {
+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<StackSpaceRegion>(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<PathSensitiveBugReport>(BT, ErrorMsg, N);
+
+ // Track the argument.
+ bugreporter::trackExpressionValue(Report->getErrorNode(), ArgExpr, *Report);
+
+ C.emitReport(std::move(Report));
+}
+
+void ento::registerPutenvWithAuto(CheckerManager &Mgr) {
+ Mgr.registerChecker<PutenvWithAutoChecker>();
+}
+
+bool ento::shouldRegisterPutenvWithAuto(const LangOptions &) { return true; }
diff --git a/clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp b/clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
index bdae3e605eff..a601370775b4 100644
--- 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
index 000000000000..d982fcb8a1ba
--- /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 at -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
index 000000000000..f2bd7b393d88
--- /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 at -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
More information about the cfe-commits
mailing list