Skip to content

Commit 6a554ac

Browse files
author
Nikita Kraiouchkine
committed
EXP43-C: Refine data-flow and add context to output
1 parent 009fc65 commit 6a554ac

File tree

6 files changed

+85
-45
lines changed

6 files changed

+85
-45
lines changed

‎c/cert/src/rules/EXP43-C/DoNotPassAliasedPointerToRestrictQualifiedParam.ql

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ class CallToFunctionWithRestrictParameters extends FunctionCall {
4949
.getIndex())
5050
}
5151

52-
Expr getAPtrArg() {
53-
result = this.getAnArgument() and
52+
Expr getAPtrArg(int index) {
53+
result = this.getArgument(index) and
5454
pointerValue(result)
5555
}
5656

@@ -69,9 +69,13 @@ class CallToFunctionWithRestrictParameters extends FunctionCall {
6969
* A `PointsToExpr` that is an argument of a pointer-type in a `CallToFunctionWithRestrictParameters`
7070
*/
7171
class CallToFunctionWithRestrictParametersArgExpr extends Expr {
72+
int paramIndex;
73+
7274
CallToFunctionWithRestrictParametersArgExpr() {
73-
this = any(CallToFunctionWithRestrictParameters call).getAPtrArg()
75+
this = any(CallToFunctionWithRestrictParameters call).getAPtrArg(paramIndex)
7476
}
77+
78+
int getParamIndex() { result = paramIndex }
7579
}
7680

7781
int getStatedValue(Expr e) {
@@ -101,28 +105,41 @@ class PointerValueToRestrictArgConfig extends DataFlow::Configuration {
101105

102106
override predicate isSink(DataFlow::Node sink) {
103107
exists(CallToFunctionWithRestrictParameters call |
104-
sink.asExpr() = call.getAPtrArg().getAChild*()
108+
sink.asExpr() = call.getAPtrArg(_).getAChild*()
105109
)
106110
}
111+
112+
override predicate isBarrierIn(DataFlow::Node node) {
113+
exists(AddressOfExpr a | node.asExpr() = a.getOperand().getAChild*())
114+
}
107115
}
108116

109117
from
110118
CallToFunctionWithRestrictParameters call, CallToFunctionWithRestrictParametersArgExpr arg1,
111-
CallToFunctionWithRestrictParametersArgExpr arg2, int argOffset1, int argOffset2
119+
CallToFunctionWithRestrictParametersArgExpr arg2, int argOffset1, int argOffset2, Expr source1,
120+
Expr source2, string sourceMessage1, string sourceMessage2
112121
where
113122
not isExcluded(call, Pointers3Package::doNotPassAliasedPointerToRestrictQualifiedParamQuery()) and
114123
arg1 = call.getARestrictPtrArg() and
115-
arg2 = call.getAPtrArg() and
116-
arg1 != arg2 and
117-
exists(PointerValueToRestrictArgConfig config, Expr source1, Expr source2 |
124+
arg2 = call.getAPtrArg(_) and
125+
// enforce ordering to remove permutations if multiple restrict-qualified args exist
126+
(not arg2 = call.getARestrictPtrArg() or arg2.getParamIndex() > arg1.getParamIndex()) and
127+
// check if two pointers address the same object
128+
exists(PointerValueToRestrictArgConfig config |
118129
config.hasFlow(DataFlow::exprNode(source1), DataFlow::exprNode(arg1.getAChild*())) and
119130
(
120131
// one pointer value flows to both args
121-
config.hasFlow(DataFlow::exprNode(source1), DataFlow::exprNode(arg2.getAChild*()))
132+
config.hasFlow(DataFlow::exprNode(source1), DataFlow::exprNode(arg2.getAChild*())) and
133+
sourceMessage1 = "$@" and
134+
sourceMessage2 = "source" and
135+
source1 = source2
122136
or
123137
// there are two separate values that flow from an AddressOfExpr of the same target
124138
getAddressOfExprTargetBase(source1) = getAddressOfExprTargetBase(source2) and
125-
config.hasFlow(DataFlow::exprNode(source2), DataFlow::exprNode(arg2.getAChild*()))
139+
config.hasFlow(DataFlow::exprNode(source2), DataFlow::exprNode(arg2.getAChild*())) and
140+
sourceMessage1 = "a pair of address-of expressions ($@, $@)" and
141+
sourceMessage2 = "addressof1" and
142+
not source1 = source2
126143
)
127144
) and
128145
// get the offset of the pointer arithmetic operand (or '0' if there is none)
@@ -146,5 +163,6 @@ where
146163
not exists(call.getAPossibleSizeArg())
147164
)
148165
select call,
149-
"Call to '" + call.getTarget().getName() +
150-
"' passes an aliased pointer to a restrict-qualified parameter."
166+
"Call to '" + call.getTarget().getName() + "' passes an $@ to a $@ (pointer value derived from " +
167+
sourceMessage1 + ".", arg2, "aliased pointer", arg1, "restrict-qualified parameter", source1,
168+
sourceMessage2, source2, "addressof2"

‎c/cert/src/rules/EXP43-C/RestrictPointerReferencesOverlappingObject.ql

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ class AssignmentOrInitializationToRestrictPtrValueExpr extends Expr {
2828
}
2929

3030
Variable getVariable() { result = v }
31+
32+
predicate isTargetRestrictQualifiedAndInSameScope() {
33+
this.(VariableAccess).getTarget().getType().hasSpecifier("restrict") and
34+
this.(VariableAccess).getTarget().getParentScope() = this.getVariable().getParentScope()
35+
}
3136
}
3237

3338
/**
@@ -46,30 +51,41 @@ class AssignedValueToRestrictPtrValueConfiguration extends DataFlow::Configurati
4651
override predicate isSink(DataFlow::Node sink) {
4752
sink.asExpr() instanceof AssignmentOrInitializationToRestrictPtrValueExpr
4853
}
54+
55+
override predicate isBarrierIn(DataFlow::Node node) {
56+
isSource(node)
57+
}
4958
}
5059

5160
from
52-
AssignedValueToRestrictPtrValueConfiguration config, DataFlow::Node sourceValue,
53-
AssignmentOrInitializationToRestrictPtrValueExpr expr,
54-
AssignmentOrInitializationToRestrictPtrValueExpr pre_expr
61+
AssignedValueToRestrictPtrValueConfiguration config,
62+
AssignmentOrInitializationToRestrictPtrValueExpr expr, DataFlow::Node sourceValue,
63+
string sourceMessage
5564
where
5665
not isExcluded(expr, Pointers3Package::restrictPointerReferencesOverlappingObjectQuery()) and
5766
(
67+
// Two restrict-qualified pointers in the same scope assigned to each other
68+
expr.isTargetRestrictQualifiedAndInSameScope() and
69+
sourceValue.asExpr() = expr and
70+
sourceMessage = "the object pointed to by " + expr.(VariableAccess).getTarget().getName()
71+
or
5872
// If the same expressions flows to two unique `AssignmentOrInitializationToRestrictPtrValueExpr`
5973
// in the same block, then the two variables point to the same (overlapping) object
60-
expr.getEnclosingBlock() = pre_expr.getEnclosingBlock() and
61-
(
62-
config.hasFlow(sourceValue, DataFlow::exprNode(pre_expr)) and
63-
config.hasFlow(sourceValue, DataFlow::exprNode(expr))
64-
or
65-
// Expressions referring to the address of the same variable can also result in aliasing
66-
getAddressOfExprTargetBase(expr) = getAddressOfExprTargetBase(pre_expr)
67-
) and
68-
strictlyDominates(pragma[only_bind_out](pre_expr), pragma[only_bind_out](expr))
69-
or
70-
// Two restrict-qualified pointers in the same scope assigned to each other
71-
expr.(VariableAccess).getTarget().getType().hasSpecifier("restrict") and
72-
expr.(VariableAccess).getTarget().getParentScope() = expr.getVariable().getParentScope()
74+
not expr.isTargetRestrictQualifiedAndInSameScope() and
75+
exists(AssignmentOrInitializationToRestrictPtrValueExpr pre_expr |
76+
expr.getEnclosingBlock() = pre_expr.getEnclosingBlock() and
77+
(
78+
config.hasFlow(sourceValue, DataFlow::exprNode(pre_expr)) and
79+
config.hasFlow(sourceValue, DataFlow::exprNode(expr)) and
80+
sourceMessage = "the same source value"
81+
or
82+
// Expressions referring to the address of the same variable can also result in aliasing
83+
getAddressOfExprTargetBase(expr) = getAddressOfExprTargetBase(pre_expr) and
84+
sourceValue.asExpr() = pre_expr and
85+
sourceMessage = getAddressOfExprTargetBase(expr).getName() + " via address-of"
86+
) and
87+
strictlyDominates(pragma[only_bind_out](pre_expr), pragma[only_bind_out](expr))
88+
)
7389
)
74-
select expr, "Assignment to restrict-qualified pointer $@ results in pointer aliasing.",
75-
expr.getVariable(), expr.getVariable().getName()
90+
select expr, "Assignment to restrict-qualified pointer $@ results in pointers aliasing $@.",
91+
expr.getVariable(), expr.getVariable().getName(), sourceValue, sourceMessage

‎c/cert/test/rules/EXP43-C/DoNotPassAliasedPointerToRestrictQualifiedParam.expected

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
| test.c:59:3:59:6 | call to copy | Call to 'copy' passes an aliased pointer to a restrict-qualified parameter. |
2-
| test.c:64:3:64:6 | call to copy | Call to 'copy' passes an aliased pointer to a restrict-qualified parameter. |
3-
| test.c:70:3:70:8 | call to strcpy | Call to 'strcpy' passes an aliased pointer to a restrict-qualified parameter. |
4-
| test.c:77:3:77:8 | call to memcpy | Call to 'memcpy' passes an aliased pointer to a restrict-qualified parameter. |
5-
| test.c:90:3:90:7 | call to scanf | Call to 'scanf' passes an aliased pointer to a restrict-qualified parameter. |
1+
| test.c:59:3:59:6 | call to copy | Call to 'copy' passes an $@ to a $@ (pointer value derived from a pair of address-of expressions ($@, $@). | test.c:59:13:59:15 | & ... | aliased pointer | test.c:59:8:59:10 | & ... | restrict-qualified parameter | test.c:59:8:59:10 | & ... | addressof1 | test.c:59:13:59:15 | & ... | addressof2 |
2+
| test.c:65:3:65:6 | call to copy | Call to 'copy' passes an $@ to a $@ (pointer value derived from a pair of address-of expressions ($@, $@). | test.c:65:15:65:19 | & ... | aliased pointer | test.c:65:8:65:12 | & ... | restrict-qualified parameter | test.c:65:8:65:12 | & ... | addressof1 | test.c:65:15:65:19 | & ... | addressof2 |
3+
| test.c:67:3:67:6 | call to copy | Call to 'copy' passes an $@ to a $@ (pointer value derived from a pair of address-of expressions ($@, $@). | test.c:67:15:67:16 | px | aliased pointer | test.c:67:8:67:12 | & ... | restrict-qualified parameter | test.c:67:8:67:12 | & ... | addressof1 | test.c:63:13:63:17 | & ... | addressof2 |
4+
| test.c:73:3:73:8 | call to strcpy | Call to 'strcpy' passes an $@ to a $@ (pointer value derived from a pair of address-of expressions ($@, $@). | test.c:73:15:73:21 | ... + ... | aliased pointer | test.c:73:10:73:12 | & ... | restrict-qualified parameter | test.c:73:10:73:12 | & ... | addressof1 | test.c:73:15:73:17 | & ... | addressof2 |
5+
| test.c:80:3:80:8 | call to memcpy | Call to 'memcpy' passes an $@ to a $@ (pointer value derived from a pair of address-of expressions ($@, $@). | test.c:80:15:80:21 | ... + ... | aliased pointer | test.c:80:10:80:12 | & ... | restrict-qualified parameter | test.c:80:10:80:12 | & ... | addressof1 | test.c:80:15:80:17 | & ... | addressof2 |
6+
| test.c:93:3:93:7 | call to scanf | Call to 'scanf' passes an $@ to a $@ (pointer value derived from a pair of address-of expressions ($@, $@). | test.c:93:14:93:20 | ... + ... | aliased pointer | test.c:93:9:93:11 | & ... | restrict-qualified parameter | test.c:93:9:93:11 | & ... | addressof1 | test.c:93:14:93:16 | & ... | addressof2 |

‎c/cert/test/rules/EXP43-C/RestrictPointerReferencesOverlappingObject.expected

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
| test.c:18:22:18:23 | i2 | Assignment to restrict-qualified pointer $@ results in pointer aliasing. | test.c:18:17:18:18 | i3 | i3 |
2-
| test.c:19:8:19:9 | g2 | Assignment to restrict-qualified pointer $@ results in pointer aliasing. | test.c:5:15:5:16 | g1 | g1 |
3-
| test.c:20:8:20:9 | i2 | Assignment to restrict-qualified pointer $@ results in pointer aliasing. | test.c:16:17:16:18 | i1 | i1 |
4-
| test.c:27:10:27:11 | g1 | Assignment to restrict-qualified pointer $@ results in pointer aliasing. | test.c:23:19:23:20 | i5 | i5 |
5-
| test.c:28:10:28:11 | g1 | Assignment to restrict-qualified pointer $@ results in pointer aliasing. | test.c:22:19:22:20 | i4 | i4 |
6-
| test.c:39:22:39:26 | & ... | Assignment to restrict-qualified pointer $@ results in pointer aliasing. | test.c:39:17:39:18 | px | px |
7-
| test.c:45:10:45:14 | & ... | Assignment to restrict-qualified pointer $@ results in pointer aliasing. | test.c:42:19:42:20 | pz | pz |
8-
| test.c:46:10:46:14 | & ... | Assignment to restrict-qualified pointer $@ results in pointer aliasing. | test.c:41:19:41:20 | py | py |
1+
| test.c:18:22:18:23 | i2 | Assignment to restrict-qualified pointer $@ results in pointers aliasing $@. | test.c:18:17:18:18 | i3 | i3 | test.c:18:22:18:23 | i2 | the object pointed to by i2 |
2+
| test.c:19:8:19:9 | g2 | Assignment to restrict-qualified pointer $@ results in pointers aliasing $@. | test.c:5:15:5:16 | g1 | g1 | test.c:19:8:19:9 | g2 | the object pointed to by g2 |
3+
| test.c:20:8:20:9 | i2 | Assignment to restrict-qualified pointer $@ results in pointers aliasing $@. | test.c:16:17:16:18 | i1 | i1 | test.c:20:8:20:9 | i2 | the object pointed to by i2 |
4+
| test.c:27:10:27:11 | g1 | Assignment to restrict-qualified pointer $@ results in pointers aliasing $@. | test.c:23:19:23:20 | i5 | i5 | test.c:19:8:19:9 | g2 | the same source value |
5+
| test.c:28:10:28:11 | g1 | Assignment to restrict-qualified pointer $@ results in pointers aliasing $@. | test.c:22:19:22:20 | i4 | i4 | test.c:19:8:19:9 | g2 | the same source value |
6+
| test.c:39:22:39:26 | & ... | Assignment to restrict-qualified pointer $@ results in pointers aliasing $@. | test.c:39:17:39:18 | px | px | test.c:38:28:38:30 | & ... | v1 via address-of |
7+
| test.c:45:10:45:14 | & ... | Assignment to restrict-qualified pointer $@ results in pointers aliasing $@. | test.c:42:19:42:20 | pz | pz | test.c:43:10:43:14 | & ... | v1 via address-of |
8+
| test.c:46:10:46:14 | & ... | Assignment to restrict-qualified pointer $@ results in pointers aliasing $@. | test.c:41:19:41:20 | py | py | test.c:43:10:43:14 | & ... | v1 via address-of |
9+
| test.c:46:10:46:14 | & ... | Assignment to restrict-qualified pointer $@ results in pointers aliasing $@. | test.c:41:19:41:20 | py | py | test.c:45:10:45:14 | & ... | v1 via address-of |

‎c/cert/test/rules/EXP43-C/test.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,11 @@ void test_restrict_params() {
6060
copy(&i1, &i2, 1); // COMPLIANT
6161

6262
int x[10];
63-
copy(&x[0], &x[1], 1); // COMPLIANT - non overlapping
64-
copy(&x[0], &x[1], 2); // NON_COMPLIANT - overlapping
63+
int *px = &x[0];
64+
copy(&x[0], &x[1], 1); // COMPLIANT - non overlapping
65+
copy(&x[0], &x[1], 2); // NON_COMPLIANT - overlapping
66+
copy(&x[0], (int *)x[0], 1); // COMPLIANT - non overlapping
67+
copy(&x[0], px, 1); // NON_COMPLIANT - overlapping
6568
}
6669

6770
void test_strcpy() {

‎c/common/src/codingstandards/c/Variable.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ class FlexibleArrayMemberCandidate extends MemberVariable {
4747
Variable getAddressOfExprTargetBase(AddressOfExpr expr) {
4848
result = expr.getOperand().(ValueFieldAccess).getQualifier().(VariableAccess).getTarget()
4949
or
50+
not expr.getOperand() instanceof ValueFieldAccess and
5051
result = expr.getOperand().(VariableAccess).getTarget()
5152
or
5253
result = expr.getOperand().(ArrayExpr).getArrayBase().(VariableAccess).getTarget()

0 commit comments

Comments
 (0)