Skip to content

Commit 19c33ab

Browse files
geoffw0geoffw0
committed
C++: Refine StrLenFunction, including removal of taint flow.
1 parent 705529c commit 19c33ab

File tree

9 files changed

+6
-49
lines changed

9 files changed

+6
-49
lines changed

‎cpp/ql/src/semmle/code/cpp/models/implementations/Pure.qll

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunction, SideE
7979
}
8080
}
8181

82-
class StrLenFunction extends AliasFunction, ArrayFunction, TaintFunction, SideEffectFunction {
82+
class StrLenFunction extends AliasFunction, ArrayFunction, SideEffectFunction {
8383
StrLenFunction() {
8484
exists(string name |
8585
hasGlobalOrStdName(name) and
@@ -107,30 +107,12 @@ class StrLenFunction extends AliasFunction, ArrayFunction, TaintFunction, SideEf
107107
getParameter(bufParam).getUnspecifiedType() instanceof PointerType
108108
}
109109

110-
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
111-
exists(ParameterIndex i |
112-
input.isParameter(i) and
113-
exists(getParameter(i))
114-
or
115-
input.isParameterDeref(i) and
116-
getParameter(i).getUnspecifiedType() instanceof PointerType
117-
) and
118-
(
119-
output.isReturnValueDeref() and
120-
getUnspecifiedType() instanceof PointerType
121-
or
122-
output.isReturnValue()
123-
)
124-
}
125-
126110
override predicate parameterNeverEscapes(int i) {
127-
getParameter(i).getUnspecifiedType() instanceof PointerType and
128-
not parameterEscapesOnlyViaReturn(i)
111+
getParameter(i).getUnspecifiedType() instanceof PointerType
129112
}
130113

131114
override predicate parameterEscapesOnlyViaReturn(int i) {
132-
i = 0 and
133-
getUnspecifiedType() instanceof PointerType
115+
none()
134116
}
135117

136118
override predicate parameterIsAlwaysReturned(int i) { none() }

‎cpp/ql/test/library-tests/dataflow/security-taint/tainted_diff.expected

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,6 @@
33
| test.cpp:49:23:49:28 | call to getenv | test.cpp:50:29:50:40 | envStrGlobal | AST only |
44
| test.cpp:49:23:49:28 | call to getenv | test.cpp:52:2:52:12 | * ... | AST only |
55
| test.cpp:49:23:49:28 | call to getenv | test.cpp:52:3:52:12 | envStr_ptr | AST only |
6-
| test.cpp:60:29:60:34 | call to getenv | test.cpp:64:10:64:14 | bytes | IR only |
7-
| test.cpp:60:29:60:34 | call to getenv | test.cpp:64:18:64:23 | call to strlen | IR only |
8-
| test.cpp:60:29:60:34 | call to getenv | test.cpp:64:18:64:37 | (int)... | IR only |
9-
| test.cpp:60:29:60:34 | call to getenv | test.cpp:64:18:64:37 | ... + ... | IR only |
106
| test.cpp:68:28:68:33 | call to getenv | test.cpp:11:20:11:21 | s1 | AST only |
117
| test.cpp:68:28:68:33 | call to getenv | test.cpp:67:7:67:13 | copying | AST only |
128
| test.cpp:68:28:68:33 | call to getenv | test.cpp:69:10:69:13 | copy | AST only |

‎cpp/ql/test/library-tests/dataflow/security-taint/tainted_ir.expected

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,6 @@
2929
| test.cpp:60:29:60:34 | call to getenv | test.cpp:60:18:60:25 | userName | |
3030
| test.cpp:60:29:60:34 | call to getenv | test.cpp:60:29:60:34 | call to getenv | |
3131
| test.cpp:60:29:60:34 | call to getenv | test.cpp:60:29:60:47 | (const char *)... | |
32-
| test.cpp:60:29:60:34 | call to getenv | test.cpp:64:10:64:14 | bytes | |
33-
| test.cpp:60:29:60:34 | call to getenv | test.cpp:64:18:64:23 | call to strlen | |
34-
| test.cpp:60:29:60:34 | call to getenv | test.cpp:64:18:64:37 | (int)... | |
35-
| test.cpp:60:29:60:34 | call to getenv | test.cpp:64:18:64:37 | ... + ... | |
3632
| test.cpp:60:29:60:34 | call to getenv | test.cpp:64:25:64:32 | userName | |
3733
| test.cpp:68:28:68:33 | call to getenv | test.cpp:11:36:11:37 | s2 | |
3834
| test.cpp:68:28:68:33 | call to getenv | test.cpp:68:17:68:24 | userName | |

‎cpp/ql/test/library-tests/dataflow/taint-tests/format.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,11 @@ void test2()
148148
wchar_t *ws = wstring::source();
149149
int i;
150150

151-
sink(strlen(s)); // [FALSE POSITIVE]
152-
sink(wcslen(ws)); // [FALSE POSITIVE]
151+
sink(strlen(s));
152+
sink(wcslen(ws));
153153

154154
i = strlen(s) + 1;
155-
sink(i); // [FALSE POSITIVE]
155+
sink(i);
156156

157157
sink(s[strlen(s) - 1]); // tainted
158158
sink(ws + (wcslen(ws) / 2)); // tainted

‎cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -118,23 +118,18 @@
118118
| format.cpp:148:16:148:30 | call to source | format.cpp:152:14:152:15 | ws | |
119119
| format.cpp:148:16:148:30 | call to source | format.cpp:158:7:158:8 | ws | |
120120
| format.cpp:148:16:148:30 | call to source | format.cpp:158:20:158:21 | ws | |
121-
| format.cpp:151:14:151:14 | s | format.cpp:151:7:151:12 | call to strlen | TAINT |
122-
| format.cpp:152:14:152:15 | ws | format.cpp:152:7:152:12 | call to wcslen | TAINT |
123121
| format.cpp:154:6:154:11 | call to strlen | format.cpp:154:6:154:18 | ... + ... | TAINT |
124122
| format.cpp:154:6:154:18 | ... + ... | format.cpp:154:2:154:18 | ... = ... | |
125123
| format.cpp:154:6:154:18 | ... + ... | format.cpp:155:7:155:7 | i | |
126-
| format.cpp:154:13:154:13 | s | format.cpp:154:6:154:11 | call to strlen | TAINT |
127124
| format.cpp:154:18:154:18 | 1 | format.cpp:154:6:154:18 | ... + ... | TAINT |
128125
| format.cpp:157:7:157:7 | s | format.cpp:157:7:157:22 | access to array | TAINT |
129126
| format.cpp:157:9:157:14 | call to strlen | format.cpp:157:9:157:21 | ... - ... | TAINT |
130127
| format.cpp:157:9:157:21 | ... - ... | format.cpp:157:7:157:22 | access to array | TAINT |
131-
| format.cpp:157:16:157:16 | s | format.cpp:157:9:157:14 | call to strlen | TAINT |
132128
| format.cpp:157:21:157:21 | 1 | format.cpp:157:9:157:21 | ... - ... | TAINT |
133129
| format.cpp:158:7:158:8 | ws | format.cpp:158:7:158:27 | ... + ... | TAINT |
134130
| format.cpp:158:7:158:27 | ref arg ... + ... | format.cpp:158:7:158:8 | ws [inner post update] | |
135131
| format.cpp:158:13:158:18 | call to wcslen | format.cpp:158:13:158:26 | ... / ... | TAINT |
136132
| format.cpp:158:13:158:26 | ... / ... | format.cpp:158:7:158:27 | ... + ... | TAINT |
137-
| format.cpp:158:20:158:21 | ws | format.cpp:158:13:158:18 | call to wcslen | TAINT |
138133
| format.cpp:158:26:158:26 | 2 | format.cpp:158:13:158:26 | ... / ... | TAINT |
139134
| stl.cpp:67:12:67:17 | call to source | stl.cpp:71:7:71:7 | a | |
140135
| stl.cpp:68:16:68:20 | 123 | stl.cpp:68:16:68:21 | call to basic_string | TAINT |

‎cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,6 @@
88
| format.cpp:100:8:100:13 | buffer | format.cpp:99:30:99:43 | call to source |
99
| format.cpp:105:8:105:13 | buffer | format.cpp:104:31:104:45 | call to source |
1010
| format.cpp:110:8:110:14 | wbuffer | format.cpp:109:38:109:52 | call to source |
11-
| format.cpp:151:7:151:12 | call to strlen | format.cpp:147:12:147:25 | call to source |
12-
| format.cpp:152:7:152:12 | call to wcslen | format.cpp:148:16:148:30 | call to source |
13-
| format.cpp:155:7:155:7 | i | format.cpp:147:12:147:25 | call to source |
1411
| format.cpp:157:7:157:22 | access to array | format.cpp:147:12:147:25 | call to source |
1512
| format.cpp:158:7:158:27 | ... + ... | format.cpp:148:16:148:30 | call to source |
1613
| stl.cpp:71:7:71:7 | a | stl.cpp:67:12:67:17 | call to source |

‎cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.expected

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
| format.cpp:151:7:151:12 | call to strlen | format.cpp:147:12:147:25 | call to source |
2-
| format.cpp:152:7:152:12 | call to wcslen | format.cpp:148:16:148:30 | call to source |
3-
| format.cpp:155:7:155:7 | i | format.cpp:147:12:147:25 | call to source |
41
| format.cpp:157:7:157:22 | (int)... | format.cpp:147:12:147:25 | call to source |
52
| format.cpp:157:7:157:22 | access to array | format.cpp:147:12:147:25 | call to source |
63
| format.cpp:158:7:158:27 | ... + ... | format.cpp:148:16:148:30 | call to source |

‎cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/ArithmeticTainted.expected

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,3 @@
88
| test.c:14:15:14:28 | maxConnections | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.c:11:29:11:32 | argv | User-provided value |
99
| test.c:44:7:44:10 | len2 | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.c:41:17:41:20 | argv | User-provided value |
1010
| test.c:54:7:54:10 | len3 | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.c:51:17:51:20 | argv | User-provided value |
11-
| test.c:74:7:74:10 | len5 | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.c:71:19:71:22 | argv | User-provided value |
12-
| test.c:84:7:84:10 | len6 | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.c:81:19:81:22 | argv | User-provided value |
13-
| test.c:94:7:94:10 | len7 | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.c:91:19:91:22 | argv | User-provided value |

‎cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/IntegerOverflowTainted.expected

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,3 @@
44
| test5.cpp:10:9:10:15 | call to strtoul | $@ flows to here and is used in an expression which might overflow. | test5.cpp:9:7:9:9 | buf | User-provided value |
55
| test.c:44:7:44:12 | ... -- | $@ flows to here and is used in an expression which might overflow negatively. | test.c:41:17:41:20 | argv | User-provided value |
66
| test.c:54:7:54:12 | ... -- | $@ flows to here and is used in an expression which might overflow negatively. | test.c:51:17:51:20 | argv | User-provided value |
7-
| test.c:74:7:74:12 | ... -- | $@ flows to here and is used in an expression which might overflow negatively. | test.c:71:19:71:22 | argv | User-provided value |
8-
| test.c:84:7:84:12 | ... -- | $@ flows to here and is used in an expression which might overflow negatively. | test.c:81:19:81:22 | argv | User-provided value |
9-
| test.c:94:7:94:12 | ... -- | $@ flows to here and is used in an expression which might overflow negatively. | test.c:91:19:91:22 | argv | User-provided value |

0 commit comments

Comments
 (0)