Skip to content

Commit 0c28310

Browse files
author
Nikita Kraiouchkine
committed
Update EXP39-C test and implement query
1 parent 03bac50 commit 0c28310

5 files changed

+288
-3
lines changed
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# EXP39-C: Do not access a variable through a pointer of an incompatible type
2+
3+
This query implements the CERT-C rule EXP39-C:
4+
5+
> Do not access a variable through a pointer of an incompatible type
6+
7+
8+
## CERT
9+
10+
** REPLACE THIS BY RUNNING THE SCRIPT `scripts/help/cert-help-extraction.py` **
11+
12+
## Implementation notes
13+
14+
None
15+
16+
## References
17+
18+
* CERT-C: [EXP39-C: Do not access a variable through a pointer of an incompatible type](https://wiki.sei.cmu.edu/confluence/display/c)
Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
/**
2+
* @id c/cert/do-not-access-variable-via-pointer-of-incompatible-type
3+
* @name EXP39-C: Do not access a variable through a pointer of an incompatible type
4+
* @description Modifying underlying pointer data through a pointer of an incompatible type can lead
5+
* to unpredictable results.
6+
* @kind path-problem
7+
* @precision very-high
8+
* @problem.severity error
9+
* @tags external/cert/id/exp39-c
10+
* correctness
11+
* external/cert/obligation/rule
12+
*/
13+
14+
import cpp
15+
import codingstandards.c.cert
16+
import semmle.code.cpp.dataflow.DataFlow
17+
import semmle.code.cpp.controlflow.Dominance
18+
import DataFlow::PathGraph
19+
20+
/**
21+
* The standard function `memset` and its assorted variants
22+
*/
23+
class MemsetFunction extends Function {
24+
MemsetFunction() {
25+
this.hasGlobalOrStdOrBslName("memset")
26+
or
27+
this.hasGlobalOrStdName("wmemset")
28+
or
29+
this.hasGlobalName(["__builtin_memset", "__builtin___memset_chk", "__builtin_memset_chk"])
30+
}
31+
}
32+
33+
class IndirectCastAnalysisUnconvertedCastExpr extends Expr {
34+
IndirectCastAnalysisUnconvertedCastExpr() { this = any(Cast c).getUnconverted() }
35+
}
36+
37+
class IndirectCastAnalysisDereferenceSink extends Expr {
38+
IndirectCastAnalysisDereferenceSink() { dereferenced(this) }
39+
}
40+
41+
class ReallocationFunction extends AllocationFunction {
42+
ReallocationFunction() { exists(this.getReallocPtrArg()) }
43+
}
44+
45+
/**
46+
* A data-flow state for a pointer which has not been reallocated.
47+
*/
48+
class IndirectCastDefaultFlowState extends DataFlow::FlowState {
49+
IndirectCastDefaultFlowState() { this = "IndirectCastDefaultFlowState" }
50+
}
51+
52+
/**
53+
* A data-flow state for a pointer which has been reallocated but
54+
* has not yet been zeroed with a memset call.
55+
*/
56+
class IndirectCastReallocatedFlowState extends DataFlow::FlowState {
57+
IndirectCastReallocatedFlowState() { this = "IndirectCastReallocatedFlowState" }
58+
}
59+
60+
/**
61+
* A data-flow configuration to track the flow from cast expressions to either
62+
* other cast expressions or to dereferences of pointers reallocated with a call
63+
* to `realloc` but not cleared via a function call to `memset`.
64+
*/
65+
class IndirectCastConfiguration extends DataFlow::Configuration {
66+
IndirectCastConfiguration() { this = "CastToIncompatibleTypeConfiguration" }
67+
68+
override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) {
69+
state instanceof IndirectCastDefaultFlowState and
70+
source.asExpr() instanceof IndirectCastAnalysisUnconvertedCastExpr
71+
}
72+
73+
override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) {
74+
sink.asExpr() instanceof IndirectCastAnalysisUnconvertedCastExpr and
75+
state instanceof IndirectCastDefaultFlowState
76+
or
77+
sink.asExpr() instanceof IndirectCastAnalysisDereferenceSink and
78+
state instanceof IndirectCastReallocatedFlowState and
79+
// The memset call won't always have an edge to subsequent dereferences.
80+
//
81+
// Therefore, check that:
82+
// 1) The memset call dominates the dereference.
83+
// 2) The realloc call dominates the memset call.
84+
// 3) There is no subsequent memset that also dominates the dereference.
85+
//
86+
// Currently, there is no relation between the pointer passed to memset
87+
// and the pointer dereferenced. This unimplemented check might produce
88+
// false-negatives when the memset call is unrelated to the reallocated memory.
89+
not exists(FunctionCall memset, FunctionCall realloc, Expr ptr |
90+
memset.getTarget() instanceof MemsetFunction and
91+
realloc.getTarget() instanceof ReallocationFunction and
92+
ptr = sink.asExpr() and
93+
dominates(memset, ptr) and
94+
not dominates(memset,
95+
any(FunctionCall other |
96+
other.getTarget() instanceof MemsetFunction and
97+
other != memset and
98+
dominates(other, ptr)
99+
|
100+
other
101+
)) and
102+
dominates(realloc, memset)
103+
)
104+
}
105+
106+
override predicate isBarrier(DataFlow::Node node, DataFlow::FlowState state) {
107+
state instanceof IndirectCastReallocatedFlowState and
108+
exists(FunctionCall fc |
109+
fc.getTarget() instanceof MemsetFunction and
110+
fc.getArgument(0) = node.asExpr()
111+
)
112+
}
113+
114+
override predicate isAdditionalFlowStep(
115+
DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
116+
DataFlow::FlowState state2
117+
) {
118+
// track pointer flow through realloc calls and update state to `IndirectCastReallocatedFlowState`
119+
state1 instanceof IndirectCastDefaultFlowState and
120+
state2 instanceof IndirectCastReallocatedFlowState and
121+
exists(FunctionCall fc |
122+
fc.getTarget() instanceof ReallocationFunction and
123+
node1.asExpr() = fc.getArgument(fc.getTarget().(ReallocationFunction).getReallocPtrArg()) and
124+
node2.asExpr() = fc
125+
)
126+
or
127+
// track pointer flow through memset calls and reset state to `IndirectCastDefaultFlowState`
128+
state1 instanceof IndirectCastReallocatedFlowState and
129+
state2 instanceof IndirectCastDefaultFlowState and
130+
exists(FunctionCall fc |
131+
fc.getTarget() instanceof MemsetFunction and
132+
node1.asExpr() = fc.getArgument(0) and
133+
node2.asExpr() = fc
134+
)
135+
}
136+
}
137+
138+
pragma[inline]
139+
predicate areTypesSameExceptForConstSpecifiers(Type a, Type b) {
140+
a.stripType() = b.stripType() and
141+
a.getSize() = b.getSize() and
142+
forall(Specifier s | s = a.getASpecifier() and not s.hasName("const") |
143+
b.hasSpecifier(s.getName())
144+
)
145+
}
146+
147+
pragma[inline]
148+
Type compatibleTypes(Type type) {
149+
not (
150+
type.isVolatile() and not result.isVolatile()
151+
or
152+
type.isConst() and not result.isConst()
153+
) and
154+
(
155+
(
156+
result instanceof UnsignedCharType or
157+
[result.stripTopLevelSpecifiers(), type.stripTopLevelSpecifiers()] instanceof VoidType
158+
)
159+
or
160+
not result instanceof UnsignedCharType and
161+
not result instanceof VoidType and
162+
(
163+
type.stripType() instanceof Struct and
164+
type.getUnspecifiedType() = result.getUnspecifiedType() and
165+
not type.getName() = "struct <unnamed>" and
166+
not result.getName() = "struct <unnamed>"
167+
or
168+
not type.stripType() instanceof Struct and
169+
(
170+
areTypesSameExceptForConstSpecifiers(type, result)
171+
or
172+
result.getSize() = type.getSize() and
173+
(
174+
type instanceof Enum and result instanceof IntegralOrEnumType
175+
or
176+
not type instanceof PlainCharType and
177+
(
178+
result.(IntegralType).isSigned() and type.(IntegralType).isSigned()
179+
or
180+
result.(IntegralType).isUnsigned() and type.(IntegralType).isUnsigned()
181+
)
182+
or
183+
result.(FloatingPointType).getDomain() = type.(FloatingPointType).getDomain()
184+
)
185+
or
186+
type instanceof Enum and result instanceof IntegralOrEnumType
187+
)
188+
)
189+
)
190+
}
191+
192+
from DataFlow::PathNode source, DataFlow::PathNode sink, Cast cast, Type fromType, Type toType
193+
where
194+
not isExcluded(cast, Pointers3Package::doNotAccessVariableViaPointerOfIncompatibleTypeQuery()) and
195+
cast.getFile().compiledAsC() and
196+
any(IndirectCastConfiguration config).hasFlowPath(source, sink) and
197+
// include only sinks which are not a compatible type to the associated source
198+
source.getNode().asExpr() = cast.getUnconverted() and
199+
fromType = cast.getUnconverted().getType().(PointerType).getBaseType() and
200+
toType = sink.getNode().asExpr().getActualType().(PointerType).getBaseType() and
201+
not toType = compatibleTypes(fromType)
202+
select sink.getNode().asExpr().getUnconverted(), source, sink,
203+
"Cast from " + fromType + " to " + toType + " results in an incompatible pointer base type."
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
edges
2+
| test.c:49:8:49:9 | s3 | test.c:50:8:50:9 | s1 |
3+
| test.c:60:16:60:18 | E1A | test.c:61:16:61:17 | e1 |
4+
| test.c:60:16:60:18 | E1A | test.c:65:10:65:12 | & ... |
5+
| test.c:68:22:68:22 | v | test.c:68:41:68:41 | v |
6+
| test.c:72:13:72:15 | & ... | test.c:68:22:68:22 | v |
7+
| test.c:74:13:74:15 | & ... | test.c:68:22:68:22 | v |
8+
| test.c:97:32:97:37 | call to malloc | test.c:98:40:98:41 | s2 |
9+
| test.c:97:32:97:37 | call to malloc | test.c:98:40:98:41 | s2 |
10+
| test.c:98:32:98:38 | call to realloc | test.c:99:3:99:4 | s3 |
11+
| test.c:98:32:98:38 | call to realloc | test.c:100:10:100:11 | s3 |
12+
| test.c:98:40:98:41 | s2 | test.c:98:32:98:38 | call to realloc |
13+
nodes
14+
| test.c:6:19:6:20 | & ... | semmle.label | & ... |
15+
| test.c:11:10:11:11 | & ... | semmle.label | & ... |
16+
| test.c:13:17:13:19 | & ... | semmle.label | & ... |
17+
| test.c:15:17:15:19 | & ... | semmle.label | & ... |
18+
| test.c:19:18:19:20 | & ... | semmle.label | & ... |
19+
| test.c:20:20:20:22 | & ... | semmle.label | & ... |
20+
| test.c:21:11:21:13 | & ... | semmle.label | & ... |
21+
| test.c:26:17:26:19 | & ... | semmle.label | & ... |
22+
| test.c:27:10:27:12 | & ... | semmle.label | & ... |
23+
| test.c:28:13:28:15 | & ... | semmle.label | & ... |
24+
| test.c:29:19:29:21 | & ... | semmle.label | & ... |
25+
| test.c:30:16:30:18 | & ... | semmle.label | & ... |
26+
| test.c:47:8:47:9 | s2 | semmle.label | s2 |
27+
| test.c:49:8:49:9 | s3 | semmle.label | s3 |
28+
| test.c:49:8:49:9 | s3 | semmle.label | s3 |
29+
| test.c:50:8:50:9 | s1 | semmle.label | s1 |
30+
| test.c:60:16:60:18 | E1A | semmle.label | E1A |
31+
| test.c:60:16:60:18 | E1A | semmle.label | E1A |
32+
| test.c:61:16:61:17 | e1 | semmle.label | e1 |
33+
| test.c:65:10:65:12 | & ... | semmle.label | & ... |
34+
| test.c:68:22:68:22 | v | semmle.label | v |
35+
| test.c:68:41:68:41 | v | semmle.label | v |
36+
| test.c:72:13:72:15 | & ... | semmle.label | & ... |
37+
| test.c:72:13:72:15 | & ... | semmle.label | & ... |
38+
| test.c:74:13:74:15 | & ... | semmle.label | & ... |
39+
| test.c:74:13:74:15 | & ... | semmle.label | & ... |
40+
| test.c:97:32:97:37 | call to malloc | semmle.label | call to malloc |
41+
| test.c:97:32:97:37 | call to malloc | semmle.label | call to malloc |
42+
| test.c:98:32:98:38 | call to realloc | semmle.label | call to realloc |
43+
| test.c:98:32:98:38 | call to realloc | semmle.label | call to realloc |
44+
| test.c:98:32:98:38 | call to realloc | semmle.label | call to realloc |
45+
| test.c:98:40:98:41 | s2 | semmle.label | s2 |
46+
| test.c:98:40:98:41 | s2 | semmle.label | s2 |
47+
| test.c:99:3:99:4 | s3 | semmle.label | s3 |
48+
| test.c:100:10:100:11 | s3 | semmle.label | s3 |
49+
subpaths
50+
#select
51+
| test.c:6:19:6:20 | & ... | test.c:6:19:6:20 | & ... | test.c:6:19:6:20 | & ... | Cast from float to int results in an incompatible pointer base type. |
52+
| test.c:11:10:11:11 | & ... | test.c:11:10:11:11 | & ... | test.c:11:10:11:11 | & ... | Cast from short[2] to int results in an incompatible pointer base type. |
53+
| test.c:13:17:13:19 | & ... | test.c:13:17:13:19 | & ... | test.c:13:17:13:19 | & ... | Cast from short[2] to short[4] results in an incompatible pointer base type. |
54+
| test.c:19:18:19:20 | & ... | test.c:19:18:19:20 | & ... | test.c:19:18:19:20 | & ... | Cast from char to signed char results in an incompatible pointer base type. |
55+
| test.c:29:19:29:21 | & ... | test.c:29:19:29:21 | & ... | test.c:29:19:29:21 | & ... | Cast from int to unsigned int results in an incompatible pointer base type. |
56+
| test.c:47:8:47:9 | s2 | test.c:47:8:47:9 | s2 | test.c:47:8:47:9 | s2 | Cast from struct <unnamed> to struct <unnamed> results in an incompatible pointer base type. |
57+
| test.c:49:8:49:9 | s3 | test.c:49:8:49:9 | s3 | test.c:49:8:49:9 | s3 | Cast from S1 to struct <unnamed> results in an incompatible pointer base type. |
58+
| test.c:50:8:50:9 | s1 | test.c:50:8:50:9 | s1 | test.c:50:8:50:9 | s1 | Cast from struct <unnamed> to S1 results in an incompatible pointer base type. |
59+
| test.c:68:41:68:41 | v | test.c:72:13:72:15 | & ... | test.c:68:41:68:41 | v | Cast from float to int results in an incompatible pointer base type. |
60+
| test.c:99:3:99:4 | s3 | test.c:98:40:98:41 | s2 | test.c:99:3:99:4 | s3 | Cast from S2 to S3 results in an incompatible pointer base type. |
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
rules/EXP39-C/DoNotAccessVariableViaPointerOfIncompatibleType.ql

c/cert/test/rules/EXP39-C/test.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
#include <stdlib.h>
2+
#include <string.h>
3+
14
void test_incompatible_arithmetic() {
25
float f = 0.0f;
36
int *p = (int *)&f; // NON_COMPLIANT - arithmetic types are not compatible
@@ -14,8 +17,8 @@ void test_incompatible_arithmetic() {
1417
// char may be signed or unsigned, and so is not compatible with either
1518
char c1;
1619
(signed char *)&c1; // NON_COMPLIANT
17-
(unsigned char *)&c1; // NON_COMPLIANT
18-
(char *)&c1; // NON_COMPLIANT
20+
(unsigned char *)&c1; // COMPLIANT - the underlying byte representation is always compatible
21+
(char *)&c1; // COMPLIANT - same type
1922

2023
// int is defined as signed, so is compatible with all the signed versions
2124
// (long, short etc. are similar)
@@ -24,7 +27,7 @@ void test_incompatible_arithmetic() {
2427
(int *)&i1; // COMPLIANT
2528
(signed *)&i1; // COMPLIANT
2629
(unsigned int *)&i1; // NON_COMPLIANT
27-
(const int *)&i1; // NON_COMPLIANT
30+
(const int *)&i1; // COMPLIANT - adding a const specifier is permitted
2831
}
2932

3033
struct {

0 commit comments

Comments
 (0)