[clang] [clang][analyzer] Move checker alpha.security.cert.pos.34c into security.PutenvWithAuto (PR #92424)
DonĂ¡t Nagy via cfe-commits
cfe-commits at lists.llvm.org
Fri May 17 05:29:45 PDT 2024
================
@@ -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
----------------
NagyDonat wrote:
```suggestion
// If a 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.
```
As you explained in the `FIXME` added in the documentation, this is not a false positive. It's a good idea to keep this testcase, but let's replace the old `TODO` comment with your reasoning for reporting this.
https://github.com/llvm/llvm-project/pull/92424
More information about the cfe-commits
mailing list