Skip to content

Commit 02b1774

Browse files
committed
C++: Switch from GVN to localFlow.
1 parent 3cfd1b5 commit 02b1774

File tree

3 files changed

+9
-19
lines changed

3 files changed

+9
-19
lines changed

cpp/ql/src/Security/CWE/CWE-311/CleartextTransmission.ql

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
import cpp
1515
import semmle.code.cpp.security.SensitiveExprs
1616
import semmle.code.cpp.dataflow.TaintTracking
17-
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
1817
import semmle.code.cpp.models.interfaces.FlowSource
1918
import semmle.code.cpp.commons.File
2019
import DataFlow::PathGraph
@@ -121,35 +120,30 @@ abstract class NetworkSendRecv extends FunctionCall {
121120
NetworkSendRecv() {
122121
this.getTarget() = target and
123122
// exclude calls based on the socket...
124-
not exists(GVN g |
125-
g = globalValueNumber(target.getSocketExpr(this)) and
123+
not exists(DataFlow::Node src, DataFlow::Node dest |
124+
DataFlow::localFlow(src, dest) and
125+
dest.asExpr() = target.getSocketExpr(this) and
126126
(
127127
// literal constant
128-
globalValueNumber(any(Literal l)) = g
128+
src.asExpr() instanceof Literal
129129
or
130130
// variable (such as a global) initialized to a literal constant
131131
exists(Variable v |
132132
v.getInitializer().getExpr() instanceof Literal and
133-
g = globalValueNumber(v.getAnAccess())
133+
src.asExpr() = v.getAnAccess()
134134
)
135135
or
136136
// result of a function call with literal inputs (likely constant)
137-
exists(FunctionCall fc |
138-
forex(Expr arg | arg = fc.getAnArgument() | arg instanceof Literal) and
139-
g = globalValueNumber(fc)
140-
)
137+
forex(Expr arg | arg = src.asExpr().(FunctionCall).getAnArgument() | arg instanceof Literal)
141138
or
142139
// variable called `stdin`, `stdout` or `stderr`
143-
exists(VariableAccess v |
144-
v.getTarget().getName() = ["stdin", "stdout", "stderr"] and
145-
g = globalValueNumber(v)
146-
)
140+
src.asExpr().(VariableAccess).getTarget().getName() = ["stdin", "stdout", "stderr"]
147141
or
148142
// open of `"/dev/tty"`
149143
exists(FunctionCall fc |
150144
fopenCall(fc) and
151145
fc.getAnArgument().getValue() = "/dev/tty" and
152-
g = globalValueNumber(fc)
146+
src.asExpr() = fc
153147
)
154148
// (this is not exhaustive)
155149
)

cpp/ql/test/query-tests/Security/CWE/CWE-311/semmle/tests/CleartextTransmission.expected

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ edges
9090
| test3.cpp:398:18:398:25 | password | test3.cpp:400:16:400:23 | password |
9191
| test3.cpp:398:18:398:25 | password | test3.cpp:400:33:400:40 | password |
9292
| test3.cpp:429:7:429:14 | password | test3.cpp:431:8:431:15 | password |
93-
| test3.cpp:465:8:465:15 | password | test3.cpp:474:11:474:18 | password |
9493
| test.cpp:41:23:41:43 | cleartext password! | test.cpp:48:21:48:27 | call to encrypt |
9594
| test.cpp:41:23:41:43 | cleartext password! | test.cpp:48:29:48:39 | thePassword |
9695
| test.cpp:66:23:66:43 | cleartext password! | test.cpp:76:21:76:27 | call to encrypt |
@@ -212,8 +211,6 @@ nodes
212211
| test3.cpp:400:33:400:40 | password | semmle.label | password |
213212
| test3.cpp:429:7:429:14 | password | semmle.label | password |
214213
| test3.cpp:431:8:431:15 | password | semmle.label | password |
215-
| test3.cpp:465:8:465:15 | password | semmle.label | password |
216-
| test3.cpp:474:11:474:18 | password | semmle.label | password |
217214
| test.cpp:41:23:41:43 | cleartext password! | semmle.label | cleartext password! |
218215
| test.cpp:48:21:48:27 | call to encrypt | semmle.label | call to encrypt |
219216
| test.cpp:48:29:48:39 | thePassword | semmle.label | thePassword |
@@ -245,4 +242,3 @@ subpaths
245242
| test3.cpp:341:4:341:7 | call to recv | test3.cpp:339:9:339:16 | password | test3.cpp:341:16:341:23 | password | This operation receives into 'password', which may put unencrypted sensitive data into $@ | test3.cpp:339:9:339:16 | password | password |
246243
| test3.cpp:388:3:388:6 | call to recv | test3.cpp:386:8:386:15 | password | test3.cpp:388:15:388:22 | password | This operation receives into 'password', which may put unencrypted sensitive data into $@ | test3.cpp:386:8:386:15 | password | password |
247244
| test3.cpp:431:2:431:6 | call to fgets | test3.cpp:429:7:429:14 | password | test3.cpp:431:8:431:15 | password | This operation receives into 'password', which may put unencrypted sensitive data into $@ | test3.cpp:429:7:429:14 | password | password |
248-
| test3.cpp:474:3:474:6 | call to recv | test3.cpp:465:8:465:15 | password | test3.cpp:474:11:474:18 | password | This operation receives into 'password', which may put unencrypted sensitive data into $@ | test3.cpp:465:8:465:15 | password | password |

cpp/ql/test/query-tests/Security/CWE/CWE-311/semmle/tests/test3.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,6 @@ void test_tty()
471471
f = STDIN_FILENO;
472472
}
473473

474-
recv(f, password, 256, val()); // GOOD: from terminal or stdin [FALSE POSITIVE]
474+
recv(f, password, 256, val()); // GOOD: from terminal or stdin
475475
}
476476
}

0 commit comments

Comments
 (0)