[clang] b98a594 - [clang][analyzer] Move `security.cert.env.InvalidPtr` out of `alpha` (#71912)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 24 01:03:01 PST 2023
Author: Endre Fülöp
Date: 2023-11-24T10:02:56+01:00
New Revision: b98a594977f25e555822e5ceef457f69c73cce45
URL: https://github.com/llvm/llvm-project/commit/b98a594977f25e555822e5ceef457f69c73cce45
DIFF: https://github.com/llvm/llvm-project/commit/b98a594977f25e555822e5ceef457f69c73cce45.diff
LOG: [clang][analyzer] Move `security.cert.env.InvalidPtr` out of `alpha` (#71912)
Thanks to recent improvements in #67663, InvalidPtr checker does not
emit any false positives on the following OS projects: memcached, tmux,
curl, twin, vim, openssl, sqlite, ffmpeg, postgres, tinyxml2, libwebm,
xerces, bitcoin, protobuf, qtbase, contour, acid, openrct2. (Before the
changes mentioned above, there were 27 reports, catching the `getenv`
invalidates previous `getenv` results cases. That strict behaviour is
disabled by default)
Added:
Modified:
clang/docs/analyzer/checkers.rst
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/test/Analysis/analyzer-config.c
clang/test/Analysis/cert/env31-c.c
clang/test/Analysis/cert/env34-c-cert-examples.c
clang/test/Analysis/cert/env34-c.c
clang/test/Analysis/invalid-ptr-checker.c
Removed:
################################################################################
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 40aa06724ccb75c..f7b48e64e324f00 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -755,6 +755,75 @@ security
Security related checkers.
+.. _security-cert-env-InvalidPtr:
+
+security.cert.env.InvalidPtr
+""""""""""""""""""""""""""""""""""
+
+Corresponds to SEI CERT Rules `ENV31-C <https://wiki.sei.cmu.edu/confluence/display/c/ENV31-C.+Do+not+rely+on+an+environment+pointer+following+an+operation+that+may+invalidate+it>`_ and `ENV34-C <https://wiki.sei.cmu.edu/confluence/display/c/ENV34-C.+Do+not+store+pointers+returned+by+certain+functions>`_.
+
+* **ENV31-C**:
+ Rule is about the possible problem with ``main`` function's third argument, environment pointer,
+ "envp". When environment array is modified using some modification function
+ such as ``putenv``, ``setenv`` or others, It may happen that memory is reallocated,
+ however "envp" is not updated to reflect the changes and points to old memory
+ region.
+
+* **ENV34-C**:
+ Some functions return a pointer to a statically allocated buffer.
+ Consequently, subsequent call of these functions will invalidate previous
+ pointer. These functions include: ``getenv``, ``localeconv``, ``asctime``, ``setlocale``, ``strerror``
+
+.. code-block:: c
+
+ int main(int argc, const char *argv[], const char *envp[]) {
+ if (setenv("MY_NEW_VAR", "new_value", 1) != 0) {
+ // setenv call may invalidate 'envp'
+ /* Handle error */
+ }
+ if (envp != NULL) {
+ for (size_t i = 0; envp[i] != NULL; ++i) {
+ puts(envp[i]);
+ // envp may no longer point to the current environment
+ // this program has unanticipated behavior, since envp
+ // does not reflect changes made by setenv function.
+ }
+ }
+ return 0;
+ }
+
+ void previous_call_invalidation() {
+ char *p, *pp;
+
+ p = getenv("VAR");
+ setenv("SOMEVAR", "VALUE", /*overwrite = */1);
+ // call to 'setenv' may invalidate p
+
+ *p;
+ // dereferencing invalid pointer
+ }
+
+
+The ``InvalidatingGetEnv`` option is available for treating ``getenv`` calls as
+invalidating. When enabled, the checker issues a warning if ``getenv`` is called
+multiple times and their results are used without first creating a copy.
+This level of strictness might be considered overly pedantic for the commonly
+used ``getenv`` implementations.
+
+To enable this option, use:
+``-analyzer-config security.cert.env.InvalidPtr:InvalidatingGetEnv=true``.
+
+By default, this option is set to *false*.
+
+When this option is enabled, warnings will be generated for scenarios like the
+following:
+
+.. code-block:: c
+
+ char* p = getenv("VAR");
+ char* pp = getenv("VAR2"); // assumes this call can invalidate `env`
+ strlen(p); // warns about accessing invalid ptr
+
.. _security-FloatLoopCounter:
security.FloatLoopCounter (C)
@@ -2549,75 +2618,6 @@ alpha.security.cert.env
SEI CERT checkers of `Environment C coding rules <https://wiki.sei.cmu.edu/confluence/x/JdcxBQ>`_.
-.. _alpha-security-cert-env-InvalidPtr:
-
-alpha.security.cert.env.InvalidPtr
-""""""""""""""""""""""""""""""""""
-
-Corresponds to SEI CERT Rules ENV31-C and ENV34-C.
-
-ENV31-C:
-Rule is about the possible problem with `main` function's third argument, environment pointer,
-"envp". When environment array is modified using some modification function
-such as putenv, setenv or others, It may happen that memory is reallocated,
-however "envp" is not updated to reflect the changes and points to old memory
-region.
-
-ENV34-C:
-Some functions return a pointer to a statically allocated buffer.
-Consequently, subsequent call of these functions will invalidate previous
-pointer. These functions include: getenv, localeconv, asctime, setlocale, strerror
-
-.. code-block:: c
-
- int main(int argc, const char *argv[], const char *envp[]) {
- if (setenv("MY_NEW_VAR", "new_value", 1) != 0) {
- // setenv call may invalidate 'envp'
- /* Handle error */
- }
- if (envp != NULL) {
- for (size_t i = 0; envp[i] != NULL; ++i) {
- puts(envp[i]);
- // envp may no longer point to the current environment
- // this program has unanticipated behavior, since envp
- // does not reflect changes made by setenv function.
- }
- }
- return 0;
- }
-
- void previous_call_invalidation() {
- char *p, *pp;
-
- p = getenv("VAR");
- setenv("SOMEVAR", "VALUE", /*overwrite = */1);
- // call to 'setenv' may invalidate p
-
- *p;
- // dereferencing invalid pointer
- }
-
-
-The ``InvalidatingGetEnv`` option is available for treating getenv calls as
-invalidating. When enabled, the checker issues a warning if getenv is called
-multiple times and their results are used without first creating a copy.
-This level of strictness might be considered overly pedantic for the commonly
-used getenv implementations.
-
-To enable this option, use:
-``-analyzer-config alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=true``.
-
-By default, this option is set to *false*.
-
-When this option is enabled, warnings will be generated for scenarios like the
-following:
-
-.. code-block:: c
-
- char* p = getenv("VAR");
- char* pp = getenv("VAR2"); // assumes this call can invalidate `env`
- strlen(p); // warns about accessing invalid ptr
-
alpha.security.taint
^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 7590a6d2b7522e5..1d224786372e823 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -71,10 +71,12 @@ 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 CERT : Package<"cert">, ParentPackage<Security>;
def ENV : Package<"env">, ParentPackage<CERT>;
+def CERTAlpha : Package<"cert">, ParentPackage<SecurityAlpha>;
+def POSAlpha : Package<"pos">, ParentPackage<CERTAlpha>;
+
def Unix : Package<"unix">;
def UnixAlpha : Package<"unix">, ParentPackage<Alpha>;
def CString : Package<"cstring">, ParentPackage<Unix>;
@@ -993,15 +995,6 @@ 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 = ENV in {
def InvalidPtrChecker : Checker<"InvalidPtr">,
@@ -1013,11 +1006,20 @@ let ParentPackage = ENV in {
"standard), which can lead to false positives depending on "
"implementation.",
"false",
- InAlpha>,
+ Released>,
]>,
Documentation<HasDocumentation>;
-} // end "alpha.cert.env"
+} // end "security.cert.env"
+
+let ParentPackage = POSAlpha 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 {
diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c
index 13b529c5d7a294a..373017f4b18bfcd 100644
--- a/clang/test/Analysis/analyzer-config.c
+++ b/clang/test/Analysis/analyzer-config.c
@@ -11,7 +11,6 @@
// CHECK-NEXT: alpha.osx.cocoa.DirectIvarAssignment:AnnotatedFunctions = false
// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04
// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01
-// CHECK-NEXT: alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv = false
// CHECK-NEXT: alpha.security.taint.TaintPropagation:Config = ""
// CHECK-NEXT: apply-fixits = false
// CHECK-NEXT: assume-controlled-environment = false
@@ -116,6 +115,7 @@
// CHECK-NEXT: region-store-small-array-limit = 5
// CHECK-NEXT: region-store-small-struct-limit = 2
// CHECK-NEXT: report-in-main-source-file = false
+// CHECK-NEXT: security.cert.env.InvalidPtr:InvalidatingGetEnv = false
// CHECK-NEXT: serialize-stats = false
// CHECK-NEXT: silence-checkers = ""
// CHECK-NEXT: stable-report-filename = false
diff --git a/clang/test/Analysis/cert/env31-c.c b/clang/test/Analysis/cert/env31-c.c
index feaa2baefea7c72..f7abc717229ea92 100644
--- a/clang/test/Analysis/cert/env31-c.c
+++ b/clang/test/Analysis/cert/env31-c.c
@@ -1,25 +1,25 @@
// RUN: %clang_analyze_cc1 -analyzer-output=text -Wno-unused %s \
-// RUN: -analyzer-checker=core,alpha.security.cert.env.InvalidPtr \
+// RUN: -analyzer-checker=core,security.cert.env.InvalidPtr \
// RUN: -verify=putenv,common \
// RUN: -DENV_INVALIDATING_CALL="putenv(\"X=Y\")"
//
// RUN: %clang_analyze_cc1 -analyzer-output=text -Wno-unused %s \
-// RUN: -analyzer-checker=core,alpha.security.cert.env.InvalidPtr \
+// RUN: -analyzer-checker=core,security.cert.env.InvalidPtr \
// RUN: -verify=putenvs,common \
// RUN: -DENV_INVALIDATING_CALL="_putenv_s(\"X\", \"Y\")"
//
// RUN: %clang_analyze_cc1 -analyzer-output=text -Wno-unused %s \
-// RUN: -analyzer-checker=core,alpha.security.cert.env.InvalidPtr \
+// RUN: -analyzer-checker=core,security.cert.env.InvalidPtr \
// RUN: -verify=wputenvs,common \
// RUN: -DENV_INVALIDATING_CALL="_wputenv_s(\"X\", \"Y\")"
//
// RUN: %clang_analyze_cc1 -analyzer-output=text -Wno-unused %s \
-// RUN: -analyzer-checker=core,alpha.security.cert.env.InvalidPtr \
+// RUN: -analyzer-checker=core,security.cert.env.InvalidPtr \
// RUN: -verify=setenv,common \
// RUN: -DENV_INVALIDATING_CALL="setenv(\"X\", \"Y\", 0)"
//
// RUN: %clang_analyze_cc1 -analyzer-output=text -Wno-unused %s \
-// RUN: -analyzer-checker=core,alpha.security.cert.env.InvalidPtr \
+// RUN: -analyzer-checker=core,security.cert.env.InvalidPtr \
// RUN: -verify=unsetenv,common \
// RUN: -DENV_INVALIDATING_CALL="unsetenv(\"X\")"
diff --git a/clang/test/Analysis/cert/env34-c-cert-examples.c b/clang/test/Analysis/cert/env34-c-cert-examples.c
index 6d6659e55d86b93..ca82844fd5f7e1e 100644
--- a/clang/test/Analysis/cert/env34-c-cert-examples.c
+++ b/clang/test/Analysis/cert/env34-c-cert-examples.c
@@ -1,18 +1,18 @@
// Default options.
-// RUN: %clang_analyze_cc1 \
-// RUN: -analyzer-checker=core,alpha.security.cert.env.InvalidPtr \
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=core,security.cert.env.InvalidPtr \
// RUN: -verify -Wno-unused %s
//
// Test the laxer handling of getenv function (this is the default).
-// RUN: %clang_analyze_cc1 \
-// RUN: -analyzer-checker=core,alpha.security.cert.env.InvalidPtr \
-// RUN: -analyzer-config alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=false \
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=core,security.cert.env.InvalidPtr \
+// RUN: -analyzer-config security.cert.env.InvalidPtr:InvalidatingGetEnv=false \
// RUN: -verify -Wno-unused %s
//
// Test the stricter handling of getenv function.
-// RUN: %clang_analyze_cc1 \
-// RUN: -analyzer-checker=core,alpha.security.cert.env.InvalidPtr \
-// RUN: -analyzer-config alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=true \
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=core,security.cert.env.InvalidPtr \
+// RUN: -analyzer-config security.cert.env.InvalidPtr:InvalidatingGetEnv=true \
// RUN: -verify=expected,pedantic -Wno-unused %s
#include "../Inputs/system-header-simulator.h"
diff --git a/clang/test/Analysis/cert/env34-c.c b/clang/test/Analysis/cert/env34-c.c
index 94bc2cf95ccc9b0..d307f0d8f4bb01f 100644
--- a/clang/test/Analysis/cert/env34-c.c
+++ b/clang/test/Analysis/cert/env34-c.c
@@ -1,6 +1,6 @@
// RUN: %clang_analyze_cc1 \
-// RUN: -analyzer-checker=alpha.security.cert.env.InvalidPtr\
-// RUN: -analyzer-config alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=true \
+// RUN: -analyzer-checker=security.cert.env.InvalidPtr\
+// RUN: -analyzer-config security.cert.env.InvalidPtr:InvalidatingGetEnv=true \
// RUN: -analyzer-output=text -verify -Wno-unused %s
#include "../Inputs/system-header-simulator.h"
diff --git a/clang/test/Analysis/invalid-ptr-checker.c b/clang/test/Analysis/invalid-ptr-checker.c
index e8ffee7fb479dc9..81bee58aac208f2 100644
--- a/clang/test/Analysis/invalid-ptr-checker.c
+++ b/clang/test/Analysis/invalid-ptr-checker.c
@@ -1,12 +1,12 @@
-// RUN: %clang_analyze_cc1 \
-// RUN: -analyzer-checker=alpha.security.cert.env.InvalidPtr \
-// RUN: -analyzer-config alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=false \
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=security.cert.env.InvalidPtr \
+// RUN: -analyzer-config security.cert.env.InvalidPtr:InvalidatingGetEnv=false \
// RUN: -analyzer-output=text -verify -Wno-unused %s
//
-// RUN: %clang_analyze_cc1 \
-// RUN: -analyzer-checker=alpha.security.cert.env.InvalidPtr \
-// RUN: -analyzer-config \
-// RUN: alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=true \
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=security.cert.env.InvalidPtr \
+// RUN: -analyzer-config \
+// RUN: security.cert.env.InvalidPtr:InvalidatingGetEnv=true \
// RUN: -analyzer-output=text -verify=expected,pedantic -Wno-unused %s
#include "Inputs/system-header-simulator.h"
More information about the cfe-commits
mailing list