Skip to content

Commit 51a5b77

Browse files
t-rasmudt-rasmud
authored
[Webkit Checkers] Introduce a Webkit checker for memory unsafe casts (#114606)
This PR introduces a new checker `[alpha.webkit.MemoryUnsafeCastChecker]` that warns all downcasts from a base type to a derived type. rdar://137766829
1 parent cb6a02a commit 51a5b77

File tree

6 files changed

+548
-0
lines changed

6 files changed

+548
-0
lines changed

‎clang/docs/analyzer/checkers.rst

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3438,6 +3438,31 @@ alpha.WebKit
34383438
34393439
.. _alpha-webkit-NoUncheckedPtrMemberChecker:
34403440
3441+
alpha.webkit.MemoryUnsafeCastChecker
3442+
""""""""""""""""""""""""""""""""""""""
3443+
Check for all casts from a base type to its derived type as these might be memory-unsafe.
3444+
3445+
Example:
3446+
3447+
.. code-block:: cpp
3448+
3449+
class Base { };
3450+
class Derived : public Base { };
3451+
3452+
void f(Base* base) {
3453+
Derived* derived = static_cast<Derived*>(base); // ERROR
3454+
}
3455+
3456+
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.
3457+
3458+
This applies to:
3459+
3460+
- C structs, C++ structs and classes, and Objective-C classes and protocols.
3461+
- Pointers and references.
3462+
- Inside template instantiations and macro expansions that are visible to the compiler.
3463+
3464+
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.
3465+
34413466
alpha.webkit.NoUncheckedPtrMemberChecker
34423467
""""""""""""""""""""""""""""""""""""""""
34433468
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.

‎clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1749,6 +1749,10 @@ def UncountedLambdaCapturesChecker : Checker<"UncountedLambdaCapturesChecker">,
17491749

17501750
let ParentPackage = WebKitAlpha in {
17511751

1752+
def MemoryUnsafeCastChecker : Checker<"MemoryUnsafeCastChecker">,
1753+
HelpText<"Check for memory unsafe casts from base type to derived type.">,
1754+
Documentation<HasDocumentation>;
1755+
17521756
def NoUncheckedPtrMemberChecker : Checker<"NoUncheckedPtrMemberChecker">,
17531757
HelpText<"Check for no unchecked member variables.">,
17541758
Documentation<HasDocumentation>;

‎clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ add_clang_library(clangStaticAnalyzerCheckers
131131
VirtualCallChecker.cpp
132132
WebKit/RawPtrRefMemberChecker.cpp
133133
WebKit/ASTUtils.cpp
134+
WebKit/MemoryUnsafeCastChecker.cpp
134135
WebKit/PtrTypesSemantics.cpp
135136
WebKit/RefCntblBaseVirtualDtorChecker.cpp
136137
WebKit/RawPtrRefCallArgsChecker.cpp

‎clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
//=======- MemoryUnsafeCastChecker.cpp -------------------------*- C++ -*-==//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
//
9+
// This file defines MemoryUnsafeCast checker, which checks for casts from a
10+
// base type to a derived type.
11+
//===----------------------------------------------------------------------===//
12+
13+
#include "clang/ASTMatchers/ASTMatchFinder.h"
14+
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
15+
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
16+
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
17+
#include "clang/StaticAnalyzer/Core/Checker.h"
18+
#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
19+
20+
using namespace clang;
21+
using namespace ento;
22+
using namespace ast_matchers;
23+
24+
namespace {
25+
static constexpr const char *const BaseNode = "BaseNode";
26+
static constexpr const char *const DerivedNode = "DerivedNode";
27+
static constexpr const char *const FromCastNode = "FromCast";
28+
static constexpr const char *const ToCastNode = "ToCast";
29+
static constexpr const char *const WarnRecordDecl = "WarnRecordDecl";
30+
31+
class MemoryUnsafeCastChecker : public Checker<check::ASTCodeBody> {
32+
BugType BT{this, "Unsafe cast", "WebKit coding guidelines"};
33+
34+
public:
35+
void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
36+
BugReporter &BR) const;
37+
};
38+
} // end namespace
39+
40+
static void emitDiagnostics(const BoundNodes &Nodes, BugReporter &BR,
41+
AnalysisDeclContext *ADC,
42+
const MemoryUnsafeCastChecker *Checker,
43+
const BugType &BT) {
44+
const auto *CE = Nodes.getNodeAs<CastExpr>(WarnRecordDecl);
45+
const NamedDecl *Base = Nodes.getNodeAs<NamedDecl>(BaseNode);
46+
const NamedDecl *Derived = Nodes.getNodeAs<NamedDecl>(DerivedNode);
47+
assert(CE && Base && Derived);
48+
49+
std::string Diagnostics;
50+
llvm::raw_string_ostream OS(Diagnostics);
51+
OS << "Unsafe cast from base type '" << Base->getNameAsString()
52+
<< "' to derived type '" << Derived->getNameAsString() << "'";
53+
PathDiagnosticLocation BSLoc(CE->getSourceRange().getBegin(),
54+
BR.getSourceManager());
55+
auto Report = std::make_unique<BasicBugReport>(BT, OS.str(), BSLoc);
56+
Report->addRange(CE->getSourceRange());
57+
BR.emitReport(std::move(Report));
58+
}
59+
60+
static void emitDiagnosticsUnrelated(const BoundNodes &Nodes, BugReporter &BR,
61+
AnalysisDeclContext *ADC,
62+
const MemoryUnsafeCastChecker *Checker,
63+
const BugType &BT) {
64+
const auto *CE = Nodes.getNodeAs<CastExpr>(WarnRecordDecl);
65+
const NamedDecl *FromCast = Nodes.getNodeAs<NamedDecl>(FromCastNode);
66+
const NamedDecl *ToCast = Nodes.getNodeAs<NamedDecl>(ToCastNode);
67+
assert(CE && FromCast && ToCast);
68+
69+
std::string Diagnostics;
70+
llvm::raw_string_ostream OS(Diagnostics);
71+
OS << "Unsafe cast from type '" << FromCast->getNameAsString()
72+
<< "' to an unrelated type '" << ToCast->getNameAsString() << "'";
73+
PathDiagnosticLocation BSLoc(CE->getSourceRange().getBegin(),
74+
BR.getSourceManager());
75+
auto Report = std::make_unique<BasicBugReport>(BT, OS.str(), BSLoc);
76+
Report->addRange(CE->getSourceRange());
77+
BR.emitReport(std::move(Report));
78+
}
79+
80+
namespace clang {
81+
namespace ast_matchers {
82+
AST_MATCHER_P(StringLiteral, mentionsBoundType, std::string, BindingID) {
83+
return Builder->removeBindings([this, &Node](const BoundNodesMap &Nodes) {
84+
const auto &BN = Nodes.getNode(this->BindingID);
85+
if (const auto *ND = BN.get<NamedDecl>()) {
86+
return ND->getName() != Node.getString();
87+
}
88+
return true;
89+
});
90+
}
91+
} // end namespace ast_matchers
92+
} // end namespace clang
93+
94+
static decltype(auto) hasTypePointingTo(DeclarationMatcher DeclM) {
95+
return hasType(pointerType(pointee(hasDeclaration(DeclM))));
96+
}
97+
98+
void MemoryUnsafeCastChecker::checkASTCodeBody(const Decl *D,
99+
AnalysisManager &AM,
100+
BugReporter &BR) const {
101+
102+
AnalysisDeclContext *ADC = AM.getAnalysisDeclContext(D);
103+
104+
// Match downcasts from base type to derived type and warn
105+
auto MatchExprPtr = allOf(
106+
hasSourceExpression(hasTypePointingTo(cxxRecordDecl().bind(BaseNode))),
107+
hasTypePointingTo(cxxRecordDecl(isDerivedFrom(equalsBoundNode(BaseNode)))
108+
.bind(DerivedNode)),
109+
unless(anyOf(hasSourceExpression(cxxThisExpr()),
110+
hasTypePointingTo(templateTypeParmDecl()))));
111+
auto MatchExprPtrObjC = allOf(
112+
hasSourceExpression(ignoringImpCasts(hasType(objcObjectPointerType(
113+
pointee(hasDeclaration(objcInterfaceDecl().bind(BaseNode))))))),
114+
ignoringImpCasts(hasType(objcObjectPointerType(pointee(hasDeclaration(
115+
objcInterfaceDecl(isDerivedFrom(equalsBoundNode(BaseNode)))
116+
.bind(DerivedNode)))))));
117+
auto MatchExprRefTypeDef =
118+
allOf(hasSourceExpression(hasType(hasUnqualifiedDesugaredType(recordType(
119+
hasDeclaration(decl(cxxRecordDecl().bind(BaseNode))))))),
120+
hasType(hasUnqualifiedDesugaredType(recordType(hasDeclaration(
121+
decl(cxxRecordDecl(isDerivedFrom(equalsBoundNode(BaseNode)))
122+
.bind(DerivedNode)))))),
123+
unless(anyOf(hasSourceExpression(hasDescendant(cxxThisExpr())),
124+
hasType(templateTypeParmDecl()))));
125+
126+
auto ExplicitCast = explicitCastExpr(anyOf(MatchExprPtr, MatchExprRefTypeDef,
127+
MatchExprPtrObjC))
128+
.bind(WarnRecordDecl);
129+
auto Cast = stmt(ExplicitCast);
130+
131+
auto Matches =
132+
match(stmt(forEachDescendant(Cast)), *D->getBody(), AM.getASTContext());
133+
for (BoundNodes Match : Matches)
134+
emitDiagnostics(Match, BR, ADC, this, BT);
135+
136+
// Match casts between unrelated types and warn
137+
auto MatchExprPtrUnrelatedTypes = allOf(
138+
hasSourceExpression(
139+
hasTypePointingTo(cxxRecordDecl().bind(FromCastNode))),
140+
hasTypePointingTo(cxxRecordDecl().bind(ToCastNode)),
141+
unless(anyOf(hasTypePointingTo(cxxRecordDecl(
142+
isSameOrDerivedFrom(equalsBoundNode(FromCastNode)))),
143+
hasSourceExpression(hasTypePointingTo(cxxRecordDecl(
144+
isSameOrDerivedFrom(equalsBoundNode(ToCastNode))))))));
145+
auto MatchExprPtrObjCUnrelatedTypes = allOf(
146+
hasSourceExpression(ignoringImpCasts(hasType(objcObjectPointerType(
147+
pointee(hasDeclaration(objcInterfaceDecl().bind(FromCastNode))))))),
148+
ignoringImpCasts(hasType(objcObjectPointerType(
149+
pointee(hasDeclaration(objcInterfaceDecl().bind(ToCastNode)))))),
150+
unless(anyOf(
151+
ignoringImpCasts(hasType(
152+
objcObjectPointerType(pointee(hasDeclaration(objcInterfaceDecl(
153+
isSameOrDerivedFrom(equalsBoundNode(FromCastNode)))))))),
154+
hasSourceExpression(ignoringImpCasts(hasType(
155+
objcObjectPointerType(pointee(hasDeclaration(objcInterfaceDecl(
156+
isSameOrDerivedFrom(equalsBoundNode(ToCastNode))))))))))));
157+
auto MatchExprRefTypeDefUnrelated = allOf(
158+
hasSourceExpression(hasType(hasUnqualifiedDesugaredType(recordType(
159+
hasDeclaration(decl(cxxRecordDecl().bind(FromCastNode))))))),
160+
hasType(hasUnqualifiedDesugaredType(
161+
recordType(hasDeclaration(decl(cxxRecordDecl().bind(ToCastNode)))))),
162+
unless(anyOf(
163+
hasType(hasUnqualifiedDesugaredType(
164+
recordType(hasDeclaration(decl(cxxRecordDecl(
165+
isSameOrDerivedFrom(equalsBoundNode(FromCastNode)))))))),
166+
hasSourceExpression(hasType(hasUnqualifiedDesugaredType(
167+
recordType(hasDeclaration(decl(cxxRecordDecl(
168+
isSameOrDerivedFrom(equalsBoundNode(ToCastNode))))))))))));
169+
170+
auto ExplicitCastUnrelated =
171+
explicitCastExpr(anyOf(MatchExprPtrUnrelatedTypes,
172+
MatchExprPtrObjCUnrelatedTypes,
173+
MatchExprRefTypeDefUnrelated))
174+
.bind(WarnRecordDecl);
175+
auto CastUnrelated = stmt(ExplicitCastUnrelated);
176+
auto MatchesUnrelatedTypes = match(stmt(forEachDescendant(CastUnrelated)),
177+
*D->getBody(), AM.getASTContext());
178+
for (BoundNodes Match : MatchesUnrelatedTypes)
179+
emitDiagnosticsUnrelated(Match, BR, ADC, this, BT);
180+
}
181+
182+
void ento::registerMemoryUnsafeCastChecker(CheckerManager &Mgr) {
183+
Mgr.registerChecker<MemoryUnsafeCastChecker>();
184+
}
185+
186+
bool ento::shouldRegisterMemoryUnsafeCastChecker(const CheckerManager &mgr) {
187+
return true;
188+
}

0 commit comments

Comments
 (0)