https://github.com/balazske updated
https://github.com/llvm/llvm-project/pull/92424
From 769523d392204eac6c48cb80a2282212f3edbbe4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com>
Date: Fri, 10 May 2024 17:30:23 +0200
Subject: [PATCH 1/3] [clang][analyzer] Move checker
alpha.security.cert.pos.34c into security.PutenvWithAuto .
The "cert" package looks not useful and the checker has not a meaningful name
with the old naming scheme.
Additionally tests and documentation is updated.
The checker looks good enough to be moved into non-alpha package.
---
clang/docs/analyzer/checkers.rst | 97 +++++++++----------
.../clang/StaticAnalyzer/Checkers/Checkers.td | 10 +-
.../Analysis/cert/pos34-c-fp-suppression.cpp | 51 ----------
clang/test/Analysis/cert/pos34-c.cpp | 61 ------------
clang/test/Analysis/putenv-with-auto.c | 66 +++++++++++++
5 files changed, 119 insertions(+), 166 deletions(-)
delete mode 100644 clang/test/Analysis/cert/pos34-c-fp-suppression.cpp
delete mode 100644 clang/test/Analysis/cert/pos34-c.cpp
create mode 100644 clang/test/Analysis/putenv-with-auto.c
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index eb8b58323da4d..6ea768d003378 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1179,6 +1179,54 @@ security.insecureAPI.DeprecatedOrUnsafeBufferHandling (C)
strncpy(buf, "a", 1); // warn
}
+.. _security-putenv-with-auto:
+
+security.PutenvWithAuto
+"""""""""""""""""""""""
+Finds calls to the ``putenv`` function which pass a pointer to an automatic
variable as the argument.
+Function ``putenv`` does not copy the passed string, only a pointer to the
data is stored.
+Content of an automatic variable is likely to be overwritten after returning
from the parent function.
+
+The problem can be solved by using a static variable or dynamically allocated
+memory. Even better is to avoid using ``putenv`` (it has other problems
+related to memory leaks) and use ``setenv`` instead.
+
+The check corresponds to CERT rule
+`POS34-C. Do not call putenv() with a pointer to an automatic variable as the
argument
+<https://wiki.sei.cmu.edu/confluence/display/c/POS34-C.+Do+not+call+putenv%28%29+with+a+pointer+to+an+automatic+variable+as+the+argument>`_.
+
+.. code-block:: c
+
+ int f() {
+ char[] env = "NAME=value";
+ return putenv(env); // putenv function should not be called with auto
variables
+ }
+
+Limitations:
+
+ - In specific cases one can pass automatic variables to ``putenv``,
+ but one needs to ensure that the given environment key stays
+ alive until it's removed or overwritten.
+ Since the analyzer cannot keep track if and when the string passed to
``putenv``
+ gets deallocated or overwritten, it needs to be slightly more aggressive
+ and warn for each case, leading in some cases to false-positive reports
like this:
+
+ .. code-block:: c
+
+ void baz() {
+ char env[] = "NAME=value";
+ putenv(env); // false-positive warning: putenv function should not
be called...
+ // More code...
+ // FIXME: It looks like that if one string was passed to putenv,
+ // it should not be deallocated at all, because when reading the
+ // environment variable a pointer into this string is returned.
+ // In this case, if another (or the same) thread reads variable
"NAME"
+ // at this point and does not copy the returned string, the data may
+ // become invalid.
+ putenv((char *)"NAME=anothervalue");
+ // This putenv call overwrites the previous entry, thus that can no
longer dangle.
+ } // 'env' array becomes dead only here.
+
.. _unix-checkers:
unix
@@ -2818,55 +2866,6 @@ 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
- }
-
-Limitations:
-
- - Technically, one can pass automatic variables to ``putenv``,
- but one needs to ensure that the given environment key stays
- alive until it's removed or overwritten.
- Since the analyzer cannot keep track of which envvars get overwritten
- and when, it needs to be slightly more aggressive and warn for such
- cases too, leading in some cases to false-positive reports like this:
-
- .. code-block:: c
-
- void baz() {
- char env[] = "NAME=value";
- putenv(env); // false-positive warning: putenv function should not
be called...
- // More code...
- putenv((char *)"NAME=anothervalue");
- // This putenv call overwrites the previous entry, thus that can no
longer dangle.
- } // 'env' array becomes dead only here.
-
-alpha.security.cert.env
-^^^^^^^^^^^^^^^^^^^^^^^
-
-SEI CERT checkers of `Environment C coding rules
<https://wiki.sei.cmu.edu/confluence/x/JdcxBQ>`_.
-
alpha.security.taint
^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 64414e3d37f75..56aeeace66f9d 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1011,6 +1011,11 @@ def FloatLoopCounter : Checker<"FloatLoopCounter">,
Dependencies<[SecuritySyntaxChecker]>,
Documentation<HasDocumentation>;
+ def PutenvWithAuto : Checker<"PutenvWithAuto">,
+ HelpText<"Finds calls to the 'putenv' function which pass a pointer to "
+ "an automatic variable as the argument.">,
+ Documentation<HasDocumentation>;
+
} // end "security"
let ParentPackage = ENV in {
@@ -1032,11 +1037,6 @@ let ParentPackage = ENV in {
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/cert/pos34-c-fp-suppression.cpp
b/clang/test/Analysis/cert/pos34-c-fp-suppression.cpp
deleted file mode 100644
index d982fcb8a1baf..0000000000000
--- a/clang/test/Analysis/cert/pos34-c-fp-suppression.cpp
+++ /dev/null
@@ -1,51 +0,0 @@
-// 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
deleted file mode 100644
index f2bd7b393d889..0000000000000
--- a/clang/test/Analysis/cert/pos34-c.cpp
+++ /dev/null
@@ -1,61 +0,0 @@
-// 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
diff --git a/clang/test/Analysis/putenv-with-auto.c
b/clang/test/Analysis/putenv-with-auto.c
new file mode 100644
index 0000000000000..454185f6996a1
--- /dev/null
+++ b/clang/test/Analysis/putenv-with-auto.c
@@ -0,0 +1,66 @@
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=security.PutenvWithAuto \
+// RUN: -verify %s
+
+#include "Inputs/system-header-simulator.h"
+void free(void *);
+void *malloc(size_t);
+int putenv(char *);
+int snprintf(char *, size_t, const char *, ...);
+
+int test_auto_var(const char *var) {
+ char env[1024];
+ (void)snprintf(env, sizeof(env), "TEST=%s", var);
+ return putenv(env); // expected-warning{{The 'putenv' function should not be
called with arguments that have automatic storage}}
+}
+
+int test_static_var(const char *var) {
+ static char env[1024];
+ (void)snprintf(env, sizeof(env), "TEST=%s", var);
+ return putenv(env);
+}
+
+void test_heap_memory(const char *var) {
+ const char *env_format = "TEST=%s";
+ const size_t len = strlen(var) + strlen(env_format);
+ char *env = (char *)malloc(len);
+ if (env == NULL)
+ return;
+ if (putenv(env) != 0) // no-warning: env was dynamically allocated.
+ free(env);
+}
+
+typedef struct {
+ int A;
+ char Env[1024];
+} Mem;
+
+int test_auto_var_struct() {
+ Mem mem;
+ return putenv(mem.Env); // expected-warning{{The 'putenv' function should
not be called with arguments that have automatic storage}}
+}
+
+int test_auto_var_subarray() {
+ char env[1024];
+ return putenv(env + 100); // expected-warning{{The 'putenv' function should
not be called with arguments that have automatic storage}}
+}
+
+int test_constant() {
+ char *env = "TEST";
+ return putenv(env);
+}
+
+extern char *ext_env;
+int test_extern() {
+ return putenv(ext_env); // no-warning: extern storage class.
+}
+
+void test_auto_var_reset() {
+ char env[] = "NAME=value";
+ // TODO: False Positive
+ putenv(env); // expected-warning{{The 'putenv' function should not be called
with arguments that have automatic storage}}
+ /*
+ ...
+ */
+ putenv((char *)"NAME=anothervalue");
+}
From 543dcc650e16c4c4cf4c58addadbb0c4f623d623 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com>
Date: Fri, 17 May 2024 17:55:11 +0200
Subject: [PATCH 2/3] doc and comment improvements, move back to alpha
---
clang/docs/analyzer/checkers.rst | 73 +++++++------------
.../clang/StaticAnalyzer/Checkers/Checkers.td | 22 +++---
.../Checkers/cert/PutenvWithAutoChecker.cpp | 24 +++---
...utenv-with-auto.c => putenv-stack-array.c} | 26 ++++---
4 files changed, 62 insertions(+), 83 deletions(-)
rename clang/test/Analysis/{putenv-with-auto.c => putenv-stack-array.c} (64%)
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 6ea768d003378..e67530479113a 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1179,54 +1179,6 @@ security.insecureAPI.DeprecatedOrUnsafeBufferHandling (C)
strncpy(buf, "a", 1); // warn
}
-.. _security-putenv-with-auto:
-
-security.PutenvWithAuto
-"""""""""""""""""""""""
-Finds calls to the ``putenv`` function which pass a pointer to an automatic
variable as the argument.
-Function ``putenv`` does not copy the passed string, only a pointer to the
data is stored.
-Content of an automatic variable is likely to be overwritten after returning
from the parent function.
-
-The problem can be solved by using a static variable or dynamically allocated
-memory. Even better is to avoid using ``putenv`` (it has other problems
-related to memory leaks) and use ``setenv`` instead.
-
-The check corresponds to CERT rule
-`POS34-C. Do not call putenv() with a pointer to an automatic variable as the
argument
-<https://wiki.sei.cmu.edu/confluence/display/c/POS34-C.+Do+not+call+putenv%28%29+with+a+pointer+to+an+automatic+variable+as+the+argument>`_.
-
-.. code-block:: c
-
- int f() {
- char[] env = "NAME=value";
- return putenv(env); // putenv function should not be called with auto
variables
- }
-
-Limitations:
-
- - In specific cases one can pass automatic variables to ``putenv``,
- but one needs to ensure that the given environment key stays
- alive until it's removed or overwritten.
- Since the analyzer cannot keep track if and when the string passed to
``putenv``
- gets deallocated or overwritten, it needs to be slightly more aggressive
- and warn for each case, leading in some cases to false-positive reports
like this:
-
- .. code-block:: c
-
- void baz() {
- char env[] = "NAME=value";
- putenv(env); // false-positive warning: putenv function should not
be called...
- // More code...
- // FIXME: It looks like that if one string was passed to putenv,
- // it should not be deallocated at all, because when reading the
- // environment variable a pointer into this string is returned.
- // In this case, if another (or the same) thread reads variable
"NAME"
- // at this point and does not copy the returned string, the data may
- // become invalid.
- putenv((char *)"NAME=anothervalue");
- // This putenv call overwrites the previous entry, thus that can no
longer dangle.
- } // 'env' array becomes dead only here.
-
.. _unix-checkers:
unix
@@ -2840,6 +2792,31 @@ Warn on mmap() calls that are both writable and
executable.
// code
}
+.. _alpha-security-putenv-stack-array:
+
+alpha.security.PutenvStackArray
+"""""""""""""""""""""""""""""""
+Finds calls to the ``putenv`` function which pass a pointer to a
stack-allocated
+(automatic) array as the argument. Function ``putenv`` does not copy the passed
+string, only a pointer to the data is stored and this data can be read even by
+other threads. Content of a stack-allocated array is likely to be overwritten
+after returning from the parent function.
+
+The problem can be solved by using a static array variable or dynamically
+allocated memory. Even better is to avoid using ``putenv`` (it has other
+problems related to memory leaks) and use ``setenv`` instead.
+
+The check corresponds to CERT rule
+`POS34-C. Do not call putenv() with a pointer to an automatic variable as the
argument
+<https://wiki.sei.cmu.edu/confluence/display/c/POS34-C.+Do+not+call+putenv%28%29+with+a+pointer+to+an+automatic+variable+as+the+argument>`_.
+
+.. code-block:: c
+
+ int f() {
+ char[] env = "NAME=value";
+ return putenv(env); // putenv function should not be called with
stack-allocated string
+ }
+
.. _alpha-security-ReturnPtrRange:
alpha.security.ReturnPtrRange (C)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 56aeeace66f9d..af9b58c2c0f75 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1011,11 +1011,6 @@ def FloatLoopCounter : Checker<"FloatLoopCounter">,
Dependencies<[SecuritySyntaxChecker]>,
Documentation<HasDocumentation>;
- def PutenvWithAuto : Checker<"PutenvWithAuto">,
- HelpText<"Finds calls to the 'putenv' function which pass a pointer to "
- "an automatic variable as the argument.">,
- Documentation<HasDocumentation>;
-
} // end "security"
let ParentPackage = ENV in {
@@ -1035,10 +1030,6 @@ let ParentPackage = ENV in {
} // end "security.cert.env"
-let ParentPackage = POSAlpha in {
-
-} // end "alpha.cert.pos"
-
let ParentPackage = SecurityAlpha in {
def ArrayBoundChecker : Checker<"ArrayBound">,
@@ -1049,10 +1040,6 @@ def ArrayBoundCheckerV2 : Checker<"ArrayBoundV2">,
HelpText<"Warn about buffer overflows (newer checker)">,
Documentation<HasDocumentation>;
-def ReturnPointerRangeChecker : Checker<"ReturnPtrRange">,
- HelpText<"Check for an out-of-bound pointer being returned to callers">,
- Documentation<HasDocumentation>;
-
def MallocOverflowSecurityChecker : Checker<"MallocOverflow">,
HelpText<"Check for overflows in the arguments to malloc()">,
Documentation<HasDocumentation>;
@@ -1073,6 +1060,15 @@ def MmapWriteExecChecker : Checker<"MmapWriteExec">,
]>,
Documentation<HasDocumentation>;
+def PutenvStackArray : Checker<"PutenvStackArray">,
+ HelpText<"Finds calls to the function 'putenv' which pass a pointer to "
+ "an automatic (stack-allocated) array as the argument.">,
+ Documentation<HasDocumentation>;
+
+def ReturnPointerRangeChecker : Checker<"ReturnPtrRange">,
+ HelpText<"Check for an out-of-bound pointer being returned to callers">,
+ Documentation<HasDocumentation>;
+
} // end "alpha.security"
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp
index a82f7caf16b29..e8000a0971484 100644
--- a/clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp
@@ -1,4 +1,4 @@
-//== PutenvWithAutoChecker.cpp --------------------------------- -*- C++
-*--=//
+//== PutenvStackArrayChecker.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.
@@ -6,8 +6,8 @@
//
//===----------------------------------------------------------------------===//
//
-// This file defines PutenvWithAutoChecker which finds calls of ``putenv``
-// function with automatic variable as the argument.
+// This file defines PutenvStackArrayChecker which finds calls of ``putenv``
+// function with automatic array variable as the argument.
// https://wiki.sei.cmu.edu/confluence/x/6NYxBQ
//
//===----------------------------------------------------------------------===//
@@ -26,9 +26,9 @@ using namespace clang;
using namespace ento;
namespace {
-class PutenvWithAutoChecker : public Checker<check::PostCall> {
+class PutenvStackArrayChecker : public Checker<check::PostCall> {
private:
- BugType BT{this, "'putenv' function should not be called with auto
variables",
+ BugType BT{this, "'putenv' called with stack-allocated string",
categories::SecurityError};
const CallDescription Putenv{CDM::CLibrary, {"putenv"}, 1};
@@ -37,8 +37,8 @@ class PutenvWithAutoChecker : public Checker<check::PostCall>
{
};
} // namespace
-void PutenvWithAutoChecker::checkPostCall(const CallEvent &Call,
- CheckerContext &C) const {
+void PutenvStackArrayChecker::checkPostCall(const CallEvent &Call,
+ CheckerContext &C) const {
if (!Putenv.matches(Call))
return;
@@ -50,7 +50,7 @@ void PutenvWithAutoChecker::checkPostCall(const CallEvent
&Call,
return;
StringRef ErrorMsg = "The 'putenv' function should not be called with "
- "arguments that have automatic storage";
+ "arrays that have automatic storage";
ExplodedNode *N = C.generateErrorNode();
auto Report = std::make_unique<PathSensitiveBugReport>(BT, ErrorMsg, N);
@@ -60,8 +60,10 @@ void PutenvWithAutoChecker::checkPostCall(const CallEvent
&Call,
C.emitReport(std::move(Report));
}
-void ento::registerPutenvWithAuto(CheckerManager &Mgr) {
- Mgr.registerChecker<PutenvWithAutoChecker>();
+void ento::registerPutenvStackArray(CheckerManager &Mgr) {
+ Mgr.registerChecker<PutenvStackArrayChecker>();
}
-bool ento::shouldRegisterPutenvWithAuto(const CheckerManager &) { return true;
}
+bool ento::shouldRegisterPutenvStackArray(const CheckerManager &) {
+ return true;
+}
diff --git a/clang/test/Analysis/putenv-with-auto.c
b/clang/test/Analysis/putenv-stack-array.c
similarity index 64%
rename from clang/test/Analysis/putenv-with-auto.c
rename to clang/test/Analysis/putenv-stack-array.c
index 454185f6996a1..fbbf93259ab85 100644
--- a/clang/test/Analysis/putenv-with-auto.c
+++ b/clang/test/Analysis/putenv-stack-array.c
@@ -1,5 +1,5 @@
// RUN: %clang_analyze_cc1 \
-// RUN: -analyzer-checker=security.PutenvWithAuto \
+// RUN: -analyzer-checker=alpha.security.PutenvStackArray \
// RUN: -verify %s
#include "Inputs/system-header-simulator.h"
@@ -11,13 +11,13 @@ int snprintf(char *, size_t, const char *, ...);
int test_auto_var(const char *var) {
char env[1024];
(void)snprintf(env, sizeof(env), "TEST=%s", var);
- return putenv(env); // expected-warning{{The 'putenv' function should not be
called with arguments that have automatic storage}}
+ return putenv(env); // expected-warning{{The 'putenv' function should not be
called with arrays that have automatic storage}}
}
int test_static_var(const char *var) {
static char env[1024];
(void)snprintf(env, sizeof(env), "TEST=%s", var);
- return putenv(env);
+ return putenv(env); // no-warning: static array is used
}
void test_heap_memory(const char *var) {
@@ -37,17 +37,17 @@ typedef struct {
int test_auto_var_struct() {
Mem mem;
- return putenv(mem.Env); // expected-warning{{The 'putenv' function should
not be called with arguments that have automatic storage}}
+ return putenv(mem.Env); // expected-warning{{The 'putenv' function should
not be called with}}
}
int test_auto_var_subarray() {
char env[1024];
- return putenv(env + 100); // expected-warning{{The 'putenv' function should
not be called with arguments that have automatic storage}}
+ return putenv(env + 100); // expected-warning{{The 'putenv' function should
not be called with}}
}
int test_constant() {
char *env = "TEST";
- return putenv(env);
+ return putenv(env); // no-warning: data is not on the stack
}
extern char *ext_env;
@@ -57,10 +57,14 @@ int test_extern() {
void test_auto_var_reset() {
char env[] = "NAME=value";
- // TODO: False Positive
- putenv(env); // expected-warning{{The 'putenv' function should not be called
with arguments that have automatic storage}}
- /*
- ...
- */
+ putenv(env); // expected-warning{{The 'putenv' function should not be called
with}}
+ // ... (do something)
+ // Even cases like this are likely a bug:
+ // It looks like that if one string was passed to putenv,
+ // it should not be deallocated at all, because when reading the
+ // environment variable a pointer into this string is returned.
+ // In this case, if another (or the same) thread reads variable "NAME"
+ // at this point and does not copy the returned string, the data may
+ // become invalid.
putenv((char *)"NAME=anothervalue");
}
From dafc83f1c670d82e5dea85054c7b29ce71ee59ea Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com>
Date: Tue, 21 May 2024 10:15:42 +0200
Subject: [PATCH 3/3] fixed documentation issues
---
clang/docs/analyzer/checkers.rst | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index e67530479113a..06cc0e8215e40 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -2794,8 +2794,8 @@ Warn on mmap() calls that are both writable and
executable.
.. _alpha-security-putenv-stack-array:
-alpha.security.PutenvStackArray
-"""""""""""""""""""""""""""""""
+alpha.security.PutenvStackArray (C)
+"""""""""""""""""""""""""""""""""""
Finds calls to the ``putenv`` function which pass a pointer to a
stack-allocated
(automatic) array as the argument. Function ``putenv`` does not copy the passed
string, only a pointer to the data is stored and this data can be read even by
@@ -2813,7 +2813,7 @@ The check corresponds to CERT rule
.. code-block:: c
int f() {
- char[] env = "NAME=value";
+ char env[] = "NAME=value";
return putenv(env); // putenv function should not be called with
stack-allocated string
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
