[clang] [Webkit Checkers] Introduce a Webkit checker for memory unsafe casts (PR #114606)

via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 1 14:22:20 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Rashmi Mudduluru (t-rasmud)

<details>
<summary>Changes</summary>

This PR introduces a new checker `[alpha.webkit.MemoryUnsafeCastChecker]` that warns all downcasts from a base type to a derived type.

rdar://137766829

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


7 Files Affected:

- (modified) clang/docs/analyzer/checkers.rst (+25) 
- (modified) clang/docs/tools/clang-formatted-files.txt (+1) 
- (modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (+4) 
- (modified) clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt (+1) 
- (added) clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp (+86) 
- (added) clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast.cpp (+151) 
- (added) clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast.mm (+29) 


``````````diff
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 87b03438e6e0b9..f01755ce7a236a 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -3452,6 +3452,31 @@ alpha.WebKit
 
 .. _alpha-webkit-NoUncheckedPtrMemberChecker:
 
+alpha.webkit.MemoryUnsafeCastChecker
+""""""""""""""""""""""""""""""""""""""
+Check for all casts from a base type to its derived type as these might be memory-unsafe.
+
+Example:
+
+.. code-block:: cpp
+
+class Base { };
+class Derived : public Base { };
+
+void f(Base* base) {
+    Derived* derived = static_cast<Derived*>(base); // ERROR
+}
+
+For all cast operations (C-style casts, static_cast, reinterpret_cast, dynamic_cast), if the source type a `Base*` and the destination type is `Derived*`, where `Derived` inherits from `Base`, the static analyzer should signal an error.
+
+This applies to:
+
+- C structs, C++ structs and classes, and Objective-C classes and protocols.
+- Pointers and references.
+- Inside template instantiations and macro expansions that are visible to the compiler.
+
+For types like this, instead of using built in casts, the programmer will use helper functions that internally perform the appropriate type check and disable static analysis.
+
 alpha.webkit.NoUncheckedPtrMemberChecker
 """"""""""""""""""""""""""""""""""""""""
 Raw pointers and references to an object which supports CheckedPtr or CheckedRef can't be used as class members. Only CheckedPtr, CheckedRef, RefPtr, or Ref are allowed.
diff --git a/clang/docs/tools/clang-formatted-files.txt b/clang/docs/tools/clang-formatted-files.txt
index 67ff085144f4de..74ab155d6174fd 100644
--- a/clang/docs/tools/clang-formatted-files.txt
+++ b/clang/docs/tools/clang-formatted-files.txt
@@ -537,6 +537,7 @@ clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
 clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
 clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
 clang/lib/StaticAnalyzer/Checkers/WebKit/DiagOutputUtils.h
+clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp
 clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
 clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
 clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 9a6b35c1b9f774..445379e88ab9e3 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1752,6 +1752,10 @@ def UncountedLambdaCapturesChecker : Checker<"UncountedLambdaCapturesChecker">,
 
 let ParentPackage = WebKitAlpha in {
 
+def MemoryUnsafeCastChecker : Checker<"MemoryUnsafeCastChecker">,
+  HelpText<"Check for memory unsafe casts from base type to derived type.">,
+  Documentation<HasDocumentation>;
+
 def NoUncheckedPtrMemberChecker : Checker<"NoUncheckedPtrMemberChecker">,
   HelpText<"Check for no unchecked member variables.">,
   Documentation<HasDocumentation>;
diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 62aa5ff7f002a9..7e987740f9ee2d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -132,6 +132,7 @@ add_clang_library(clangStaticAnalyzerCheckers
   VirtualCallChecker.cpp
   WebKit/RawPtrRefMemberChecker.cpp
   WebKit/ASTUtils.cpp
+  WebKit/MemoryUnsafeCastChecker.cpp
   WebKit/PtrTypesSemantics.cpp
   WebKit/RefCntblBaseVirtualDtorChecker.cpp
   WebKit/UncountedCallArgsChecker.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp
new file mode 100644
index 00000000000000..05a5f89d28c8fe
--- /dev/null
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp
@@ -0,0 +1,86 @@
+//=======- MemoryUnsafeCastChecker.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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines MemoryUnsafeCast checker, which checks for casts from a
+// base type to a derived type.
+//===----------------------------------------------------------------------===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+class MemoryUnsafeCastChecker : public Checker<check::PreStmt<CastExpr>> {
+  BugType BT{this, ""};
+
+public:
+  void checkPreStmt(const CastExpr *CE, CheckerContext &C) const;
+};
+} // end namespace
+
+void emitWarning(CheckerContext &C, const CastExpr &CE, const BugType &BT,
+                 QualType FromType, QualType ToType) {
+  ExplodedNode *errorNode = C.generateNonFatalErrorNode();
+  if (!errorNode)
+    return;
+  SmallString<192> Buf;
+  llvm::raw_svector_ostream OS(Buf);
+  OS << "Memory unsafe cast from base type '";
+  QualType::print(FromType.getTypePtr(), Qualifiers(), OS, C.getLangOpts(),
+                  llvm::Twine());
+  OS << "' to derived type '";
+  QualType::print(ToType.getTypePtr(), Qualifiers(), OS, C.getLangOpts(),
+                  llvm::Twine());
+  OS << "'";
+  auto R = std::make_unique<PathSensitiveBugReport>(BT, OS.str(), errorNode);
+  R->addRange(CE.getSourceRange());
+  C.emitReport(std::move(R));
+}
+
+void MemoryUnsafeCastChecker::checkPreStmt(const CastExpr *CE,
+                                           CheckerContext &C) const {
+  auto ExpCast = dyn_cast_or_null<ExplicitCastExpr>(CE);
+  if (!ExpCast)
+    return;
+
+  auto ToDerivedQualType = ExpCast->getTypeAsWritten();
+  auto *SE = CE->getSubExprAsWritten();
+  if (ToDerivedQualType->isObjCObjectPointerType()) {
+    auto FromBaseQualType = SE->getType();
+    bool IsObjCSubType =
+        !C.getASTContext().hasSameType(ToDerivedQualType, FromBaseQualType) &&
+        C.getASTContext().canAssignObjCInterfaces(
+            FromBaseQualType->getAsObjCInterfacePointerType(),
+            ToDerivedQualType->getAsObjCInterfacePointerType());
+    if (IsObjCSubType)
+      emitWarning(C, *CE, BT, FromBaseQualType, ToDerivedQualType);
+    return;
+  }
+  auto ToDerivedType = ToDerivedQualType->getPointeeCXXRecordDecl();
+  auto FromBaseType = SE->getType()->getPointeeCXXRecordDecl();
+  if (!FromBaseType)
+    FromBaseType = SE->getType()->getAsCXXRecordDecl();
+  if (!FromBaseType)
+    return;
+  if (ToDerivedType->isDerivedFrom(FromBaseType))
+    emitWarning(C, *CE, BT, SE->getType(), ToDerivedQualType);
+}
+
+void ento::registerMemoryUnsafeCastChecker(CheckerManager &Mgr) {
+  Mgr.registerChecker<MemoryUnsafeCastChecker>();
+}
+
+bool ento::shouldRegisterMemoryUnsafeCastChecker(const CheckerManager &) {
+  return true;
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast.cpp b/clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast.cpp
new file mode 100644
index 00000000000000..1a4ef6858d2180
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast.cpp
@@ -0,0 +1,151 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.MemoryUnsafeCastChecker -verify %s
+
+class Base { };
+class Derived : public Base { };
+
+void test_pointers(Base *base) {
+  Derived *derived_static = static_cast<Derived*>(base);
+  // expected-warning at -1{{Memory unsafe cast from base type 'Base *' to derived type 'Derived *'}}
+  Derived *derived_reinterpret = reinterpret_cast<Derived*>(base);
+  // expected-warning at -1{{Memory unsafe cast from base type 'Base *' to derived type 'Derived *'}}
+  Derived *derived_c = (Derived*)base;
+  // expected-warning at -1{{Memory unsafe cast from base type 'Base *' to derived type 'Derived *'}}
+}
+
+void test_refs(Base &base) {
+  Derived &derived_static = static_cast<Derived&>(base);
+  // expected-warning at -1{{Memory unsafe cast from base type 'Base' to derived type 'Derived &'}}
+  Derived &derived_reinterpret = reinterpret_cast<Derived&>(base);
+  // expected-warning at -1{{Memory unsafe cast from base type 'Base' to derived type 'Derived &'}}
+  Derived &derived_c = (Derived&)base;
+  // expected-warning at -1{{Memory unsafe cast from base type 'Base' to derived type 'Derived &'}}
+}
+
+class BaseVirtual {
+  virtual void virtual_base_function();
+};
+
+class DerivedVirtual : public BaseVirtual {
+  void virtual_base_function() override { }
+};
+
+void test_dynamic_casts(BaseVirtual *base_ptr, BaseVirtual &base_ref) {
+  DerivedVirtual *derived_dynamic_ptr = dynamic_cast<DerivedVirtual*>(base_ptr);
+  // expected-warning at -1{{Memory unsafe cast from base type 'BaseVirtual *' to derived type 'DerivedVirtual *'}}
+  DerivedVirtual &derived_dynamic_ref = dynamic_cast<DerivedVirtual&>(base_ref);
+  // expected-warning at -1{{Memory unsafe cast from base type 'BaseVirtual' to derived type 'DerivedVirtual &'}}
+}
+
+struct BaseStruct { };
+struct DerivedStruct : BaseStruct { };
+
+void test_struct_pointers(struct BaseStruct *base_struct) {
+  struct DerivedStruct *derived_static = static_cast<struct DerivedStruct*>(base_struct);
+  // expected-warning at -1{{Memory unsafe cast from base type 'struct BaseStruct *' to derived type 'struct DerivedStruct *'}}
+  struct DerivedStruct *derived_reinterpret = reinterpret_cast<struct DerivedStruct*>(base_struct);
+  // expected-warning at -1{{Memory unsafe cast from base type 'struct BaseStruct *' to derived type 'struct DerivedStruct *'}}
+  struct DerivedStruct *derived_c = (struct DerivedStruct*)base_struct;
+  // expected-warning at -1{{Memory unsafe cast from base type 'struct BaseStruct *' to derived type 'struct DerivedStruct *'}}
+}
+
+typedef struct BaseStruct BStruct;
+typedef struct DerivedStruct DStruct;
+
+void test_struct_refs(BStruct &base_struct) {
+  DStruct &derived_static = static_cast<DStruct&>(base_struct);
+  // expected-warning at -1{{Memory unsafe cast from base type 'BStruct' to derived type 'DStruct &'}}
+  DStruct &derived_reinterpret = reinterpret_cast<DStruct&>(base_struct);
+  // expected-warning at -1{{Memory unsafe cast from base type 'BStruct' to derived type 'DStruct &'}}
+  DStruct &derived_c = (DStruct&)base_struct;
+  // expected-warning at -1{{Memory unsafe cast from base type 'BStruct' to derived type 'DStruct &'}}
+}
+
+int counter = 0;
+void test_recursive(BStruct &base_struct) {
+  if (counter == 5)
+    return;
+  counter++;
+  DStruct &derived_static = static_cast<DStruct&>(base_struct);
+  // expected-warning at -1{{Memory unsafe cast from base type 'BStruct' to derived type 'DStruct &'}}
+}
+
+template<typename T>
+class BaseTemplate { };
+
+template<typename T>
+class DerivedTemplate : public BaseTemplate<T> { };
+
+void test_templates(BaseTemplate<int> *base, BaseTemplate<int> &base_ref) {
+  DerivedTemplate<int> *derived_static = static_cast<DerivedTemplate<int>*>(base);
+  // expected-warning at -1{{Memory unsafe cast from base type 'BaseTemplate<int> *' to derived type 'DerivedTemplate<int> *'}}
+  DerivedTemplate<int> *derived_reinterpret = reinterpret_cast<DerivedTemplate<int>*>(base);
+  // expected-warning at -1{{Memory unsafe cast from base type 'BaseTemplate<int> *' to derived type 'DerivedTemplate<int> *'}}
+  DerivedTemplate<int> *derived_c = (DerivedTemplate<int>*)base;
+  // expected-warning at -1{{Memory unsafe cast from base type 'BaseTemplate<int> *' to derived type 'DerivedTemplate<int> *'}}
+  DerivedTemplate<int> &derived_static_ref = static_cast<DerivedTemplate<int>&>(base_ref);
+  // expected-warning at -1{{Memory unsafe cast from base type 'BaseTemplate<int>' to derived type 'DerivedTemplate<int> &'}}
+  DerivedTemplate<int> &derived_reinterpret_ref = reinterpret_cast<DerivedTemplate<int>&>(base_ref);
+  // expected-warning at -1{{Memory unsafe cast from base type 'BaseTemplate<int>' to derived type 'DerivedTemplate<int> &'}}
+  DerivedTemplate<int> &derived_c_ref = (DerivedTemplate<int>&)base_ref;
+  // expected-warning at -1{{Memory unsafe cast from base type 'BaseTemplate<int>' to derived type 'DerivedTemplate<int> &'}}
+}
+
+#define CAST_MACRO_STATIC(X,Y) (static_cast<Y>(X))
+#define CAST_MACRO_REINTERPRET(X,Y) (reinterpret_cast<Y>(X))
+#define CAST_MACRO_C(X,Y) ((Y)X)
+
+void test_macro_static(Base *base, Derived *derived, Base &base_ref) {
+  Derived *derived_static = CAST_MACRO_STATIC(base, Derived*);
+  // expected-warning at -1{{Memory unsafe cast from base type 'Base *' to derived type 'Derived *'}}
+  Derived &derived_static_ref = CAST_MACRO_STATIC(base_ref, Derived&);
+  // expected-warning at -1{{Memory unsafe cast from base type 'Base' to derived type 'Derived &'}}
+  Base *base_static_same = CAST_MACRO_STATIC(base, Base*);  // no warning
+  Base *base_static_upcast = CAST_MACRO_STATIC(derived, Base*);  // no warning
+}
+
+void test_macro_reinterpret(Base *base, Derived *derived, Base &base_ref) {
+  Derived *derived_reinterpret = CAST_MACRO_REINTERPRET(base, Derived*);
+  // expected-warning at -1{{Memory unsafe cast from base type 'Base *' to derived type 'Derived *'}}
+  Derived &derived_reinterpret_ref = CAST_MACRO_REINTERPRET(base_ref, Derived&);
+  // expected-warning at -1{{Memory unsafe cast from base type 'Base' to derived type 'Derived &' [alpha.webkit.MemoryUnsafeCastChecker]}}
+  Base *base_reinterpret_same = CAST_MACRO_REINTERPRET(base, Base*);  // no warning
+  Base *base_reinterpret_upcast = CAST_MACRO_REINTERPRET(derived, Base*);  // no warning
+}
+
+void test_macro_c(Base *base, Derived *derived, Base &base_ref) {
+  Derived *derived_c = CAST_MACRO_C(base, Derived*);
+  // expected-warning at -1{{Memory unsafe cast from base type 'Base *' to derived type 'Derived *' [alpha.webkit.MemoryUnsafeCastChecker]}}
+  Derived &derived_c_ref = CAST_MACRO_C(base_ref, Derived&);
+  // expected-warning at -1{{Memory unsafe cast from base type 'Base' to derived type 'Derived &' [alpha.webkit.MemoryUnsafeCastChecker]}}
+  Base *base_c_same = CAST_MACRO_C(base, Base*);  // no warning
+  Base *base_c_upcast = CAST_MACRO_C(derived, Base*);  // no warning
+}
+
+struct BaseStructCpp {
+  int t;
+  void increment() { t++; }
+};
+struct DerivedStructCpp : BaseStructCpp {
+  void increment_t() {increment();}
+};
+
+void test_struct_cpp_pointers(struct BaseStructCpp *base_struct) {
+  struct DerivedStructCpp *derived_static = static_cast<struct DerivedStructCpp*>(base_struct);
+  // expected-warning at -1{{Memory unsafe cast from base type 'struct BaseStructCpp *' to derived type 'struct DerivedStructCpp *'}}
+  struct DerivedStructCpp *derived_reinterpret = reinterpret_cast<struct DerivedStructCpp*>(base_struct);
+  // expected-warning at -1{{Memory unsafe cast from base type 'struct BaseStructCpp *' to derived type 'struct DerivedStructCpp *'}}
+  struct DerivedStructCpp *derived_c = (struct DerivedStructCpp*)base_struct;
+  // expected-warning at -1{{Memory unsafe cast from base type 'struct BaseStructCpp *' to derived type 'struct DerivedStructCpp *'}}
+}
+
+typedef struct BaseStructCpp BStructCpp;
+typedef struct DerivedStructCpp DStructCpp;
+
+void test_struct_cpp_refs(BStructCpp &base_struct) {
+  DStructCpp &derived_static = static_cast<DStructCpp&>(base_struct);
+  // expected-warning at -1{{Memory unsafe cast from base type 'BStructCpp' to derived type 'DStructCpp &'}}
+  DStructCpp &derived_reinterpret = reinterpret_cast<DStructCpp&>(base_struct);
+  // expected-warning at -1{{Memory unsafe cast from base type 'BStructCpp' to derived type 'DStructCpp &'}}
+  DStructCpp &derived_c = (DStructCpp&)base_struct;
+  // expected-warning at -1{{Memory unsafe cast from base type 'BStructCpp' to derived type 'DStructCpp &'}}
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast.mm b/clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast.mm
new file mode 100644
index 00000000000000..1ea2f4aa472715
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast.mm
@@ -0,0 +1,29 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.MemoryUnsafeCastChecker -verify %s
+
+ at protocol NSObject
++alloc;
+-init;
+ at end
+
+ at interface NSObject <NSObject> {}
+ at end
+
+ at interface BaseClass : NSObject
+ at end
+
+ at interface DerivedClass : BaseClass
+-(void)testCasts:(BaseClass*)base;
+ at end
+
+ at implementation DerivedClass
+-(void)testCasts:(BaseClass*)base {
+  DerivedClass *derived = (DerivedClass*)base;
+  // expected-warning at -1{{Memory unsafe cast from base type 'BaseClass *' to derived type 'DerivedClass *'}}
+  DerivedClass *derived_static = static_cast<DerivedClass*>(base);
+  // expected-warning at -1{{Memory unsafe cast from base type 'BaseClass *' to derived type 'DerivedClass *'}}
+  DerivedClass *derived_reinterpret = reinterpret_cast<DerivedClass*>(base);
+  // expected-warning at -1{{Memory unsafe cast from base type 'BaseClass *' to derived type 'DerivedClass *'}}
+  base = (BaseClass*)derived;  // no warning
+  base = (BaseClass*)base;  // no warning
+}
+ at end

``````````

</details>


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


More information about the cfe-commits mailing list