Skip to content

Commit cb469e6

Browse files
mbaludambaluda
committed
Simplified ENV30-C
1 parent 787fb4c commit cb469e6

File tree

3 files changed

+21
-97
lines changed

3 files changed

+21
-97
lines changed

‎c/cert/src/rules/ENV30-C/DoNotModifyTheReturnValueOfCertainFunctions.ql

Lines changed: 13 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -27,26 +27,21 @@ class NotModifiableCall extends FunctionCall {
2727
}
2828

2929
/*
30-
* Returns an expression that modifies an object pointed by the return value of a call
31-
* to one of the `NotModifiableCall` functions.
30+
* An expression that modifies an object.
3231
*/
3332

34-
Expr objectModified(NotModifiableCall c) {
35-
// the pointed object is reassigned
36-
exists(AssignExpr ae |
37-
result =
38-
[ae.getLValue().(PointerDereferenceExpr), ae.getLValue().(PointerFieldAccess).getQualifier*()] and
39-
//exclude overwrite from the same function
40-
not ae.getRValue().(FunctionCall).getTarget().getName() = c.getTarget().getName() and
41-
//exclude overwriting of localeconv returned object with setlocale() with categories LC_ALL (0) LC_MONETARY (3) LC_NUMERIC (4)
42-
not (
43-
c.getTarget().getName() = "localeconv" and
44-
ae.getRValue().(FunctionCall).getTarget().getName() = "setlocale" and
45-
ae.getRValue().(FunctionCall).getArgument(0).getValue() = ["0", "3", "4"]
33+
class ObjectWrite extends Expr {
34+
ObjectWrite() {
35+
// the pointed object is reassigned
36+
exists(Expr e |
37+
e = [any(AssignExpr ae).getLValue(), any(CrementOperation co).getOperand()] and
38+
(
39+
this = e.(PointerDereferenceExpr).getOperand()
40+
or
41+
this = e.(PointerFieldAccess).getQualifier()
42+
)
4643
)
47-
)
48-
or
49-
exists(CrementOperation co | result = co.getOperand())
44+
}
5045
}
5146

5247
/**
@@ -59,23 +54,13 @@ class DFConf extends DataFlow::Configuration {
5954
source.asExpr() instanceof NotModifiableCall
6055
}
6156

62-
override predicate isSink(DataFlow::Node sink) { any() }
63-
64-
// Flow through pointer dereference expressions
65-
override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
66-
exists(PointerDereferenceExpr de |
67-
de.getOperand() = node1.asExpr() and
68-
de = node2.asExpr()
69-
)
70-
}
57+
override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof ObjectWrite }
7158
}
7259

7360
from DataFlow::PathNode source, DataFlow::PathNode sink
7461
where
7562
not isExcluded(sink.getNode().asExpr(),
7663
Contracts1Package::doNotModifyTheReturnValueOfCertainFunctionsQuery()) and
77-
sink != source and
78-
sink.getNode().asExpr() = objectModified(source.getNode().asExpr()) and
7964
// the modified object comes from a call to one of the ENV functions
8065
any(DFConf d).hasFlowPath(source, sink)
8166
select sink.getNode(), source, sink,

‎c/cert/test/rules/ENV30-C/DoNotModifyTheReturnValueOfCertainFunctions.expected

Lines changed: 5 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,78 +1,16 @@
11
edges
2-
| test.c:5:18:5:22 | c_str | test.c:5:18:5:22 | c_str |
3-
| test.c:5:18:5:22 | c_str | test.c:5:18:5:22 | c_str |
4-
| test.c:5:18:5:22 | c_str | test.c:6:10:6:15 | * ... |
5-
| test.c:5:18:5:22 | c_str | test.c:6:11:6:15 | c_str |
6-
| test.c:5:18:5:22 | c_str | test.c:7:9:7:14 | * ... |
7-
| test.c:5:18:5:22 | c_str | test.c:7:10:7:14 | c_str |
8-
| test.c:5:18:5:22 | c_str | test.c:8:7:8:12 | * ... |
92
| test.c:5:18:5:22 | c_str | test.c:8:8:8:12 | c_str |
10-
| test.c:5:18:5:22 | c_str | test.c:10:7:10:11 | c_str |
11-
| test.c:15:16:15:21 | call to getenv | test.c:17:3:17:20 | ... = ... |
12-
| test.c:15:16:15:21 | call to getenv | test.c:17:17:17:20 | env1 |
13-
| test.c:15:16:15:21 | call to getenv | test.c:19:7:19:10 | env1 |
143
| test.c:15:16:15:21 | call to getenv | test.c:21:9:21:12 | env1 |
15-
| test.c:15:16:15:21 | call to getenv | test.c:21:9:21:12 | env1 |
16-
| test.c:21:9:21:12 | env1 | test.c:5:18:5:22 | c_str |
174
| test.c:21:9:21:12 | env1 | test.c:5:18:5:22 | c_str |
18-
| test.c:21:9:21:12 | env1 | test.c:21:9:21:12 | ref arg env1 |
19-
| test.c:28:10:28:15 | call to getenv | test.c:28:3:28:27 | ... = ... |
20-
| test.c:28:10:28:15 | call to getenv | test.c:29:7:29:10 | env2 |
21-
| test.c:28:10:28:15 | call to getenv | test.c:32:39:32:42 | env2 |
22-
| test.c:28:10:28:15 | call to getenv | test.c:36:23:36:26 | env2 |
23-
| test.c:44:10:44:15 | call to getenv | test.c:44:3:44:27 | ... = ... |
24-
| test.c:44:10:44:15 | call to getenv | test.c:45:7:45:10 | env3 |
25-
| test.c:44:10:44:15 | call to getenv | test.c:48:24:48:27 | env3 |
26-
| test.c:60:11:60:19 | call to setlocale | test.c:60:3:60:32 | ... = ... |
27-
| test.c:60:11:60:19 | call to setlocale | test.c:62:15:62:19 | conv4 |
28-
| test.c:60:11:60:19 | call to setlocale | test.c:63:5:63:9 | conv4 |
29-
| test.c:68:31:68:40 | call to localeconv | test.c:69:7:69:11 | conv5 |
30-
| test.c:68:31:68:40 | call to localeconv | test.c:76:24:76:28 | conv5 |
5+
| test.c:61:11:61:20 | call to localeconv | test.c:64:5:64:9 | conv4 |
316
nodes
327
| test.c:5:18:5:22 | c_str | semmle.label | c_str |
33-
| test.c:5:18:5:22 | c_str | semmle.label | c_str |
34-
| test.c:5:18:5:22 | c_str | semmle.label | c_str |
35-
| test.c:5:18:5:22 | c_str | semmle.label | c_str |
36-
| test.c:6:10:6:15 | * ... | semmle.label | * ... |
37-
| test.c:6:11:6:15 | c_str | semmle.label | c_str |
38-
| test.c:7:9:7:14 | * ... | semmle.label | * ... |
39-
| test.c:7:10:7:14 | c_str | semmle.label | c_str |
40-
| test.c:8:7:8:12 | * ... | semmle.label | * ... |
418
| test.c:8:8:8:12 | c_str | semmle.label | c_str |
42-
| test.c:10:7:10:11 | c_str | semmle.label | c_str |
43-
| test.c:15:16:15:21 | call to getenv | semmle.label | call to getenv |
449
| test.c:15:16:15:21 | call to getenv | semmle.label | call to getenv |
45-
| test.c:17:3:17:20 | ... = ... | semmle.label | ... = ... |
46-
| test.c:17:17:17:20 | env1 | semmle.label | env1 |
47-
| test.c:19:7:19:10 | env1 | semmle.label | env1 |
48-
| test.c:21:9:21:12 | env1 | semmle.label | env1 |
4910
| test.c:21:9:21:12 | env1 | semmle.label | env1 |
50-
| test.c:21:9:21:12 | ref arg env1 | semmle.label | ref arg env1 |
51-
| test.c:28:3:28:27 | ... = ... | semmle.label | ... = ... |
52-
| test.c:28:10:28:15 | call to getenv | semmle.label | call to getenv |
53-
| test.c:28:10:28:15 | call to getenv | semmle.label | call to getenv |
54-
| test.c:29:7:29:10 | env2 | semmle.label | env2 |
55-
| test.c:32:39:32:42 | env2 | semmle.label | env2 |
56-
| test.c:36:23:36:26 | env2 | semmle.label | env2 |
57-
| test.c:44:3:44:27 | ... = ... | semmle.label | ... = ... |
58-
| test.c:44:10:44:15 | call to getenv | semmle.label | call to getenv |
59-
| test.c:44:10:44:15 | call to getenv | semmle.label | call to getenv |
60-
| test.c:45:7:45:10 | env3 | semmle.label | env3 |
61-
| test.c:48:24:48:27 | env3 | semmle.label | env3 |
62-
| test.c:58:25:58:34 | call to localeconv | semmle.label | call to localeconv |
63-
| test.c:60:3:60:32 | ... = ... | semmle.label | ... = ... |
64-
| test.c:60:11:60:19 | call to setlocale | semmle.label | call to setlocale |
65-
| test.c:60:11:60:19 | call to setlocale | semmle.label | call to setlocale |
66-
| test.c:62:15:62:19 | conv4 | semmle.label | conv4 |
67-
| test.c:63:5:63:9 | conv4 | semmle.label | conv4 |
68-
| test.c:68:31:68:40 | call to localeconv | semmle.label | call to localeconv |
69-
| test.c:68:31:68:40 | call to localeconv | semmle.label | call to localeconv |
70-
| test.c:69:7:69:11 | conv5 | semmle.label | conv5 |
71-
| test.c:76:24:76:28 | conv5 | semmle.label | conv5 |
11+
| test.c:61:11:61:20 | call to localeconv | semmle.label | call to localeconv |
12+
| test.c:64:5:64:9 | conv4 | semmle.label | conv4 |
7213
subpaths
73-
| test.c:21:9:21:12 | env1 | test.c:5:18:5:22 | c_str | test.c:5:18:5:22 | c_str | test.c:21:9:21:12 | ref arg env1 |
74-
| test.c:21:9:21:12 | env1 | test.c:5:18:5:22 | c_str | test.c:5:18:5:22 | c_str | test.c:21:9:21:12 | ref arg env1 |
7514
#select
76-
| test.c:8:7:8:12 | * ... | test.c:15:16:15:21 | call to getenv | test.c:8:7:8:12 | * ... | The object returned by the function getenv should no be modified. |
77-
| test.c:10:7:10:11 | c_str | test.c:15:16:15:21 | call to getenv | test.c:10:7:10:11 | c_str | The object returned by the function getenv should no be modified. |
78-
| test.c:63:5:63:9 | conv4 | test.c:60:11:60:19 | call to setlocale | test.c:63:5:63:9 | conv4 | The object returned by the function setlocale should no be modified. |
15+
| test.c:8:8:8:12 | c_str | test.c:15:16:15:21 | call to getenv | test.c:8:8:8:12 | c_str | The object returned by the function getenv should no be modified. |
16+
| test.c:64:5:64:9 | conv4 | test.c:61:11:61:20 | call to localeconv | test.c:64:5:64:9 | conv4 | The object returned by the function localeconv should no be modified. |

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ void trstr(char *c_str, char orig, char rep) {
77
if (*c_str == orig) {
88
*c_str = rep; // NON_COMPLIANT
99
}
10-
++c_str; // NON_COMPLIANT
10+
++c_str;
1111
}
1212
}
1313

@@ -57,7 +57,8 @@ void f3(void) {
5757
void f4(void) {
5858
struct lconv *conv4 = localeconv();
5959

60-
conv4 = setlocale(LC_ALL, "C"); // COMPLIANT
60+
setlocale(LC_ALL, "C"); // COMPLIANT
61+
conv4 = localeconv(); // COMPLIANT
6162

6263
if ('\0' == conv4->decimal_point[0]) {
6364
conv4->decimal_point = "."; // NON_COMPLIANT

0 commit comments

Comments
 (0)