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

[Balazs Benics via cfe-commits]

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

>From 915ab37028067fb38ffa69ae5c9726bb8c971436 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Fri, 12 Apr 2024 19:07:49 +0200
Subject: [PATCH 1/2] [analyzer] Harden security.cert.env.InvalidPtr checker fn
 matching

Fixes #88181

I'm also hardening an llvm::cast along the way.
---
 .../Checkers/cert/InvalidPtrChecker.cpp       | 31 ++++++++++++-------
 clang/test/Analysis/invalid-ptr-checker.cpp   | 10 ++++++
 2 files changed, 30 insertions(+), 11 deletions(-)
 create mode 100644 clang/test/Analysis/invalid-ptr-checker.cpp

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

>From 06d0056efd9616f76680ec7d923ed2bc76f2ab25 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Sat, 13 Apr 2024 20:44:47 +0200
Subject: [PATCH 2/2] [docs] Add release notes for the crash fix

---
 clang/docs/ReleaseNotes.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 232de0d7d8bb73..7cb550cef62e64 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -600,6 +600,8 @@ Static Analyzer
   but not under any case blocks if ``unroll-loops=true`` analyzer config is
   set. (#GH68819)
 - Support C++23 static operator calls. (#GH84972)
+- Fixed a crash in ``security.cert.env.InvalidPtr`` checker when accidentally
+  matched user-defined ``strerror`` and similar library functions. (GH#88181)
 
 New features
 ^^^^^^^^^^^^