[clang] [analyzer] Move security.cert.env.InvalidPtr out of alpha (PR #71912)
Endre Fülöp via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 10 01:29:55 PST 2023
https://github.com/gamesh411 created https://github.com/llvm/llvm-project/pull/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.
>From 2d94271affd27c5ebf1073a9effbe6c7815f5c01 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Fri, 10 Nov 2023 10:08:58 +0100
Subject: [PATCH] [analyzer] Move security.cert.env.InvalidPtr out of alpha
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
---
clang/docs/analyzer/checkers.rst | 138 +++++++++---------
.../clang/StaticAnalyzer/Checkers/Checkers.td | 28 ++--
clang/test/Analysis/analyzer-config.c | 2 +-
clang/test/Analysis/cert/env31-c.c | 10 +-
.../Analysis/cert/env34-c-cert-examples.c | 10 +-
clang/test/Analysis/cert/env34-c.c | 4 +-
clang/test/Analysis/invalid-ptr-checker.c | 8 +-
7 files changed, 101 insertions(+), 99 deletions(-)
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 43137f4b020f9f7..ff4559aa89d96a0 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 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 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)
@@ -2479,75 +2548,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 b6e9f0fae1c7f48..34edda022375cab 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>;
@@ -989,15 +991,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">,
@@ -1009,11 +1002,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 8abfbf84983d811..45adbb0348d1864 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: alpha.unix.Errno:AllowErrnoReadOutsideConditionExpressions = true
// CHECK-NEXT: apply-fixits = false
@@ -117,6 +116,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..32908c5576cb854 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..54d9a3a62c89d6a 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: -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: -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: -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..b12af9806e1daf5 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: -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-checker=security.cert.env.InvalidPtr \
// RUN: -analyzer-config \
-// RUN: alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=true \
+// 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