diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 40aa06724ccb7..f7b48e64e324f 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) @@ -2549,75 +2618,6 @@ alpha.security.cert.env SEI CERT checkers of `Environment C coding rules `_. -.. _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 7590a6d2b7522..1d224786372e8 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; def SecurityAlpha : Package<"security">, ParentPackage; def Taint : Package<"taint">, ParentPackage; -def CERT : Package<"cert">, ParentPackage; -def POS : Package<"pos">, ParentPackage; +def CERT : Package<"cert">, ParentPackage; def ENV : Package<"env">, ParentPackage; +def CERTAlpha : Package<"cert">, ParentPackage; +def POSAlpha : Package<"pos">, ParentPackage; + def Unix : Package<"unix">; def UnixAlpha : Package<"unix">, ParentPackage; def CString : Package<"cstring">, ParentPackage; @@ -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; - -} // 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; -} // 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; + +} // 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 13b529c5d7a29..373017f4b18bf 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 feaa2baefea7c..f7abc717229ea 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 6d6659e55d86b..ca82844fd5f7e 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 94bc2cf95ccc9..d307f0d8f4bb0 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 e8ffee7fb479d..81bee58aac208 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"