Skip to content

Commit 1a42e55

Browse files
authored
Merge pull request github#15820 from MathiasVP/add-type-confusion-query
C++: Add a new query for detecting type confusion vulnerabilities
2 parents 9c51514 + 7b0df57 commit 1a42e55

File tree

9 files changed

+665
-0
lines changed

9 files changed

+665
-0
lines changed
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>
6+
Certain casts in C and C++ place no restrictions on the target type. For
7+
example, C style casts such as <code>(MyClass*)p</code> allows the programmer
8+
to cast any pointer <code>p</code> to an expression of type <code>MyClass*</code>.
9+
If the runtime type of <code>p</code> turns out to be a type that's incompatible
10+
with <code>MyClass</code>, this results in undefined behavior.
11+
</p>
12+
</overview>
13+
14+
<recommendation>
15+
<p>
16+
If possible, use <code>dynamic_cast</code> to safely cast between polymorphic types.
17+
If <code>dynamic_cast</code> is not an option, use <code>static_cast</code> to restrict
18+
the kinds of conversions that the compiler is allowed to perform. If C++ style casts is
19+
not an option, carefully check that all casts are safe.
20+
</p>
21+
</recommendation>
22+
23+
<example>
24+
<p>
25+
Consider the following class hierachy where we define a base class <code>Shape</code> and two
26+
derived classes <code>Circle</code> and <code>Square</code> that are mutually incompatible:
27+
</p>
28+
<sample src="TypeConfusionCommon.cpp"/>
29+
30+
<p>
31+
The following code demonstrates a type confusion vulnerability where the programmer
32+
assumes that the runtime type of <code>p</code> is always a <code>Square</code>.
33+
However, if <code>p</code> is a <code>Circle</code>, the cast will result in undefined behavior.
34+
</p>
35+
<sample src="TypeConfusionBad.cpp"/>
36+
37+
<p>
38+
The following code fixes the vulnerability by using <code>dynamic_cast</code> to
39+
safely cast between polymorphic types. If the cast fails, <code>dynamic_cast</code>
40+
returns a null pointer, which can be checked for and handled appropriately.
41+
</p>
42+
<sample src="TypeConfusionGood.cpp"/>
43+
</example>
44+
45+
<references>
46+
<li>
47+
Microsoft Learn: <a href="https://learn.microsoft.com/en-us/cpp/cpp/type-conversions-and-type-safety-modern-cpp">Type conversions and type safety</a>.
48+
</li>
49+
</references>
50+
</qhelp>
Lines changed: 263 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,263 @@
1+
/**
2+
* @name Type confusion
3+
* @description Casting a value to an incompatible type can lead to undefined behavior.
4+
* @kind path-problem
5+
* @problem.severity warning
6+
* @security-severity 9.3
7+
* @precision medium
8+
* @id cpp/type-confusion
9+
* @tags security
10+
* external/cwe/cwe-843
11+
*/
12+
13+
import cpp
14+
import semmle.code.cpp.dataflow.new.DataFlow
15+
import Flow::PathGraph
16+
17+
/**
18+
* Holds if `f` is a field located at byte offset `offset` in `c`.
19+
*
20+
* Note that predicate is recursive, so that given the following:
21+
* ```cpp
22+
* struct S1 {
23+
* int a;
24+
* void* b;
25+
* };
26+
*
27+
* struct S2 {
28+
* S1 s1;
29+
* char c;
30+
* };
31+
* ```
32+
* both `hasAFieldWithOffset(S2, s1, 0)` and `hasAFieldWithOffset(S2, a, 0)`
33+
* holds.
34+
*/
35+
predicate hasAFieldWithOffset(Class c, Field f, int offset) {
36+
// Base case: `f` is a field in `c`.
37+
f = c.getAField() and
38+
offset = f.getByteOffset() and
39+
not f.getUnspecifiedType().(Class).hasDefinition()
40+
or
41+
// Otherwise, we find the struct that is a field of `c` which then has
42+
// the field `f` as a member.
43+
exists(Field g |
44+
g = c.getAField() and
45+
// Find the field with the largest offset that's less than or equal to
46+
// offset. That's the struct we need to search recursively.
47+
g =
48+
max(Field cand, int candOffset |
49+
cand = c.getAField() and
50+
candOffset = cand.getByteOffset() and
51+
offset >= candOffset
52+
|
53+
cand order by candOffset
54+
) and
55+
hasAFieldWithOffset(g.getUnspecifiedType(), f, offset - g.getByteOffset())
56+
)
57+
}
58+
59+
/** Holds if `f` is the last field of its declaring class. */
60+
predicate lastField(Field f) {
61+
exists(Class c | c = f.getDeclaringType() |
62+
f =
63+
max(Field cand, int byteOffset |
64+
cand.getDeclaringType() = c and byteOffset = f.getByteOffset()
65+
|
66+
cand order by byteOffset
67+
)
68+
)
69+
}
70+
71+
/**
72+
* Holds if there exists a field in `c2` at offset `offset` that's compatible
73+
* with `f1`.
74+
*/
75+
bindingset[f1, offset, c2]
76+
pragma[inline_late]
77+
predicate hasCompatibleFieldAtOffset(Field f1, int offset, Class c2) {
78+
exists(Field f2 | hasAFieldWithOffset(c2, f2, offset) |
79+
// Let's not deal with bit-fields for now.
80+
f2 instanceof BitField
81+
or
82+
f1.getUnspecifiedType().getSize() = f2.getUnspecifiedType().getSize()
83+
or
84+
lastField(f1) and
85+
f1.getUnspecifiedType().getSize() <= f2.getUnspecifiedType().getSize()
86+
)
87+
}
88+
89+
/**
90+
* Holds if `c1` is a prefix of `c2`.
91+
*/
92+
bindingset[c1, c2]
93+
pragma[inline_late]
94+
predicate prefix(Class c1, Class c2) {
95+
not c1.isPolymorphic() and
96+
not c2.isPolymorphic() and
97+
if c1 instanceof Union
98+
then
99+
// If it's a union we just verify that one of it's variants is compatible with the other class
100+
exists(Field f1, int offset |
101+
// Let's not deal with bit-fields for now.
102+
not f1 instanceof BitField and
103+
hasAFieldWithOffset(c1, f1, offset)
104+
|
105+
hasCompatibleFieldAtOffset(f1, offset, c2)
106+
)
107+
else
108+
forall(Field f1, int offset |
109+
// Let's not deal with bit-fields for now.
110+
not f1 instanceof BitField and
111+
hasAFieldWithOffset(c1, f1, offset)
112+
|
113+
hasCompatibleFieldAtOffset(f1, offset, c2)
114+
)
115+
}
116+
117+
/**
118+
* An unsafe cast is any explicit cast that is not
119+
* a `dynamic_cast`.
120+
*/
121+
class UnsafeCast extends Cast {
122+
private Class toType;
123+
124+
UnsafeCast() {
125+
(
126+
this instanceof CStyleCast
127+
or
128+
this instanceof StaticCast
129+
or
130+
this instanceof ReinterpretCast
131+
) and
132+
toType = this.getExplicitlyConverted().getUnspecifiedType().stripType() and
133+
not this.isImplicit() and
134+
exists(TypeDeclarationEntry tde |
135+
tde = toType.getDefinition() and
136+
not tde.isFromUninstantiatedTemplate(_)
137+
)
138+
}
139+
140+
Class getConvertedType() { result = toType }
141+
142+
/**
143+
* Holds if the result of this cast can safely be interpreted as a value of
144+
* type `t`.
145+
*
146+
* The compatibility rules are as follows:
147+
*
148+
* 1. the result of `(T)x` is compatible with the type `T` for any `T`
149+
* 2. the result of `(T)x` is compatible with the type `U` for any `U` such
150+
* that `U` is a subtype of `T`, or `T` is a subtype of `U`.
151+
* 3. the result of `(T)x` is compatible with the type `U` if the list
152+
* of fields of `T` is a prefix of the list of fields of `U`.
153+
* For example, if `U` is `struct { unsigned char x; int y; };`
154+
* and `T` is `struct { unsigned char uc; };`.
155+
* 4. the result of `(T)x` is compatible with the type `U` if the list
156+
* of fields of `U` is a prefix of the list of fields of `T`.
157+
*
158+
* Condition 4 is a bit controversial, since it assumes that the additional
159+
* fields in `T` won't be accessed. This may result in some FNs.
160+
*/
161+
bindingset[this, t]
162+
pragma[inline_late]
163+
predicate compatibleWith(Type t) {
164+
// Conition 1
165+
t.stripType() = this.getConvertedType()
166+
or
167+
// Condition 3
168+
prefix(this.getConvertedType(), t.stripType())
169+
or
170+
// Condition 4
171+
prefix(t.stripType(), this.getConvertedType())
172+
or
173+
// Condition 2 (a)
174+
t.stripType().(Class).getABaseClass+() = this.getConvertedType()
175+
or
176+
// Condition 2 (b)
177+
t.stripType() = this.getConvertedType().getABaseClass+()
178+
}
179+
}
180+
181+
/**
182+
* Holds if `source` is an allocation that allocates a value of type `type`.
183+
*/
184+
predicate isSourceImpl(DataFlow::Node source, Class type) {
185+
exists(AllocationExpr alloc |
186+
alloc = source.asExpr() and
187+
type = alloc.getAllocatedElementType().stripType() and
188+
not exists(
189+
alloc
190+
.(NewOrNewArrayExpr)
191+
.getAllocator()
192+
.(OperatorNewAllocationFunction)
193+
.getPlacementArgument()
194+
)
195+
) and
196+
exists(TypeDeclarationEntry tde |
197+
tde = type.getDefinition() and
198+
not tde.isFromUninstantiatedTemplate(_)
199+
)
200+
}
201+
202+
/** A configuration describing flow from an allocation to a potentially unsafe cast. */
203+
module Config implements DataFlow::ConfigSig {
204+
predicate isSource(DataFlow::Node source) { isSourceImpl(source, _) }
205+
206+
predicate isBarrier(DataFlow::Node node) {
207+
// We disable flow through global variables to reduce FPs from infeasible paths
208+
node instanceof DataFlow::VariableNode
209+
or
210+
exists(Class c | c = node.getType().stripType() |
211+
not c.hasDefinition()
212+
or
213+
exists(TypeDeclarationEntry tde |
214+
tde = c.getDefinition() and
215+
tde.isFromUninstantiatedTemplate(_)
216+
)
217+
)
218+
}
219+
220+
predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(UnsafeCast cast).getUnconverted() }
221+
222+
int fieldFlowBranchLimit() { result = 0 }
223+
}
224+
225+
module Flow = DataFlow::Global<Config>;
226+
227+
predicate relevantType(DataFlow::Node sink, Class allocatedType) {
228+
exists(DataFlow::Node source |
229+
Flow::flow(source, sink) and
230+
isSourceImpl(source, allocatedType)
231+
)
232+
}
233+
234+
predicate isSinkImpl(
235+
DataFlow::Node sink, Class allocatedType, Type convertedType, boolean compatible
236+
) {
237+
exists(UnsafeCast cast |
238+
relevantType(sink, allocatedType) and
239+
sink.asExpr() = cast.getUnconverted() and
240+
convertedType = cast.getConvertedType()
241+
|
242+
if cast.compatibleWith(allocatedType) then compatible = true else compatible = false
243+
)
244+
}
245+
246+
from
247+
Flow::PathNode source, Flow::PathNode sink, Type badSourceType, Type sinkType,
248+
DataFlow::Node sinkNode
249+
where
250+
Flow::flowPath(source, sink) and
251+
sinkNode = sink.getNode() and
252+
isSourceImpl(source.getNode(), badSourceType) and
253+
isSinkImpl(sinkNode, badSourceType, sinkType, false) and
254+
// If there is any flow that would result in a valid cast then we don't
255+
// report an alert here. This reduces the number of FPs from infeasible paths
256+
// significantly.
257+
not exists(DataFlow::Node goodSource, Type goodSourceType |
258+
isSourceImpl(goodSource, goodSourceType) and
259+
isSinkImpl(sinkNode, goodSourceType, sinkType, true) and
260+
Flow::flow(goodSource, sinkNode)
261+
)
262+
select sinkNode, source, sink, "Conversion from $@ to $@ is invalid.", badSourceType,
263+
badSourceType.toString(), sinkType, sinkType.toString()
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
void allocate_and_draw_bad() {
2+
Shape* shape = new Circle;
3+
// ...
4+
// BAD: Assumes that shape is always a Square
5+
Square* square = static_cast<Square*>(shape);
6+
int length = square->getLength();
7+
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
struct Shape {
2+
virtual ~Shape();
3+
4+
virtual void draw() = 0;
5+
};
6+
7+
struct Circle : public Shape {
8+
Circle();
9+
10+
void draw() override {
11+
/* ... */
12+
}
13+
14+
int getRadius();
15+
};
16+
17+
struct Square : public Shape {
18+
Square();
19+
20+
void draw() override {
21+
/* ... */
22+
}
23+
24+
int getLength();
25+
};
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
void allocate_and_draw_good() {
2+
Shape* shape = new Circle;
3+
// ...
4+
// GOOD: Dynamically checks if shape is a Square
5+
Square* square = dynamic_cast<Square*>(shape);
6+
if(square) {
7+
int length = square->getLength();
8+
} else {
9+
// handle error
10+
}
11+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* Added a new query, `cpp/type-confusion`, to detect casts to invalid types.

0 commit comments

Comments
 (0)