[clang] [analyzer] Harden security.cert.env.InvalidPtr checker fn matching (PR #88536)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Balazs Benics (steakhal)

<details>
<summary>Changes</summary>

Fixes #<!-- -->88181

I'm also hardening an llvm::cast along the way.

Here is the full stack trace of the original crash:
https://godbolt.org/z/jn93q39b5

---
Full diff: https://github.com/llvm/llvm-project/pull/88536.diff


2 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp (+20-11) 
- (added) clang/test/Analysis/invalid-ptr-checker.cpp (+10) 


``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
index e5dd907c660d8e..fefe846b6911f7 100644
--- a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
@@ -48,14 +48,19 @@ class InvalidPtrChecker
   bool InvalidatingGetEnv = false;
 
   // GetEnv can be treated invalidating and non-invalidating as well.
-  const CallDescription GetEnvCall{{"getenv"}, 1};
+  const CallDescription GetEnvCall{CDM::CLibrary, {"getenv"}, 1};
 
   const CallDescriptionMap<HandlerFn> EnvpInvalidatingFunctions = {
-      {{{"setenv"}, 3}, &InvalidPtrChecker::EnvpInvalidatingCall},
-      {{{"unsetenv"}, 1}, &InvalidPtrChecker::EnvpInvalidatingCall},
-      {{{"putenv"}, 1}, &InvalidPtrChecker::EnvpInvalidatingCall},
-      {{{"_putenv_s"}, 2}, &InvalidPtrChecker::EnvpInvalidatingCall},
-      {{{"_wputenv_s"}, 2}, &InvalidPtrChecker::EnvpInvalidatingCall},
+      {{CDM::CLibrary, {"setenv"}, 3},
+       &InvalidPtrChecker::EnvpInvalidatingCall},
+      {{CDM::CLibrary, {"unsetenv"}, 1},
+       &InvalidPtrChecker::EnvpInvalidatingCall},
+      {{CDM::CLibrary, {"putenv"}, 1},
+       &InvalidPtrChecker::EnvpInvalidatingCall},
+      {{CDM::CLibrary, {"_putenv_s"}, 2},
+       &InvalidPtrChecker::EnvpInvalidatingCall},
+      {{CDM::CLibrary, {"_wputenv_s"}, 2},
+       &InvalidPtrChecker::EnvpInvalidatingCall},
   };
 
   void postPreviousReturnInvalidatingCall(const CallEvent &Call,
@@ -63,13 +68,13 @@ class InvalidPtrChecker
 
   // SEI CERT ENV34-C
   const CallDescriptionMap<HandlerFn> PreviousCallInvalidatingFunctions = {
-      {{{"setlocale"}, 2},
+      {{CDM::CLibrary, {"setlocale"}, 2},
        &InvalidPtrChecker::postPreviousReturnInvalidatingCall},
-      {{{"strerror"}, 1},
+      {{CDM::CLibrary, {"strerror"}, 1},
        &InvalidPtrChecker::postPreviousReturnInvalidatingCall},
-      {{{"localeconv"}, 0},
+      {{CDM::CLibrary, {"localeconv"}, 0},
        &InvalidPtrChecker::postPreviousReturnInvalidatingCall},
-      {{{"asctime"}, 1},
+      {{CDM::CLibrary, {"asctime"}, 1},
        &InvalidPtrChecker::postPreviousReturnInvalidatingCall},
   };
 
@@ -205,8 +210,12 @@ void InvalidPtrChecker::postPreviousReturnInvalidatingCall(
       CE, LCtx, CE->getType(), C.blockCount());
   State = State->BindExpr(CE, LCtx, RetVal);
 
+  const auto *SymRegOfRetVal =
+      dyn_cast_or_null<SymbolicRegion>(RetVal.getAsRegion());
+  if (!SymRegOfRetVal)
+    return;
+
   // Remember to this region.
-  const auto *SymRegOfRetVal = cast<SymbolicRegion>(RetVal.getAsRegion());
   const MemRegion *MR = SymRegOfRetVal->getBaseRegion();
   State = State->set<PreviousCallResultMap>(FD, MR);
 
diff --git a/clang/test/Analysis/invalid-ptr-checker.cpp b/clang/test/Analysis/invalid-ptr-checker.cpp
new file mode 100644
index 00000000000000..58bb45e0fb8421
--- /dev/null
+++ b/clang/test/Analysis/invalid-ptr-checker.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,security.cert.env.InvalidPtr -verify %s
+
+// expected-no-diagnostics
+
+namespace other {
+int strerror(int errnum); // custom strerror
+void no_crash_on_custom_strerror() {
+  (void)strerror(0); // no-crash
+}
+} // namespace other

``````````

</details>


https://github.com/llvm/llvm-project/pull/88536