Skip to content

Commit e438671

Browse files
MathiasVPMathiasVP
authored
Merge pull request #14896 from MathiasVP/no-dtt-in-user-controlled-bypass
C++: Rewrite `cpp/user-controlled-bypass` away from `DefaultTaintTracking`
2 parents 631dc98 + 6b48b36 commit e438671

File tree

2 files changed

+55
-60
lines changed

2 files changed

+55
-60
lines changed

‎cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql‎

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@
1212
* external/cwe/cwe-290
1313
*/
1414

15-
import semmle.code.cpp.ir.dataflow.internal.DefaultTaintTrackingImpl
16-
import TaintedWithPath
15+
import cpp
16+
import semmle.code.cpp.dataflow.new.TaintTracking
17+
import semmle.code.cpp.security.FlowSources as FS
18+
import Flow::PathGraph
1719

1820
string getATopLevelDomain() {
1921
result =
@@ -46,6 +48,12 @@ predicate useOfHardCodedAddressOrIP(Expr use) {
4648
)
4749
}
4850

51+
Expr getExprWithoutNot(Expr expr) {
52+
result = expr and not expr instanceof NotExpr
53+
or
54+
result = getExprWithoutNot(expr.(NotExpr).getOperand()) and expr instanceof NotExpr
55+
}
56+
4957
/**
5058
* Find `IfStmt`s that have a hard-coded IP or web address in
5159
* their condition. If the condition also depends on an
@@ -57,16 +65,31 @@ predicate hardCodedAddressInCondition(Expr subexpression, Expr condition) {
5765
// One of the sub-expressions of the condition is a hard-coded
5866
// IP or web-address.
5967
exists(Expr use | use = condition.getAChild+() | useOfHardCodedAddressOrIP(use)) and
60-
condition = any(IfStmt ifStmt).getCondition()
68+
condition = getExprWithoutNot(any(IfStmt ifStmt).getCondition())
69+
}
70+
71+
predicate isSource(FS::FlowSource source, string sourceType) {
72+
source.getSourceType() = sourceType and not source instanceof DataFlow::ExprNode
6173
}
6274

63-
class Configuration extends TaintTrackingConfiguration {
64-
override predicate isSink(Element sink) { hardCodedAddressInCondition(sink, _) }
75+
predicate isSink(DataFlow::Node sink, Expr condition) {
76+
hardCodedAddressInCondition([sink.asExpr(), sink.asIndirectExpr()], condition)
6577
}
6678

67-
from Expr subexpression, Expr source, Expr condition, PathNode sourceNode, PathNode sinkNode
79+
module Config implements DataFlow::ConfigSig {
80+
predicate isSource(DataFlow::Node source) { isSource(source, _) }
81+
82+
predicate isSink(DataFlow::Node sink) { isSink(sink, _) }
83+
}
84+
85+
module Flow = TaintTracking::Global<Config>;
86+
87+
from
88+
Expr subexpression, Expr condition, Flow::PathNode source, Flow::PathNode sink, string sourceType
6889
where
6990
hardCodedAddressInCondition(subexpression, condition) and
70-
taintedWithPath(source, subexpression, sourceNode, sinkNode)
71-
select condition, sourceNode, sinkNode,
72-
"Untrusted input $@ might be vulnerable to a spoofing attack.", source, source.toString()
91+
isSource(source.getNode(), sourceType) and
92+
Flow::flowPath(source, sink) and
93+
isSink(sink.getNode(), condition)
94+
select condition, source, sink, "Untrusted input $@ might be vulnerable to a spoofing attack.",
95+
source, sourceType

‎cpp/ql/test/query-tests/Security/CWE/CWE-290/semmle/AuthenticationBypass/AuthenticationBypass.expected‎

Lines changed: 23 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,54 +1,26 @@
11
edges
2-
| test.cpp:16:25:16:30 | call to getenv | test.cpp:20:14:20:20 | address |
3-
| test.cpp:16:25:16:30 | call to getenv | test.cpp:20:14:20:20 | address |
4-
| test.cpp:16:25:16:42 | call to getenv | test.cpp:20:14:20:20 | address |
5-
| test.cpp:16:25:16:42 | call to getenv | test.cpp:20:14:20:20 | address |
6-
| test.cpp:27:25:27:30 | call to getenv | test.cpp:31:14:31:20 | address |
7-
| test.cpp:27:25:27:30 | call to getenv | test.cpp:31:14:31:20 | address |
8-
| test.cpp:27:25:27:42 | call to getenv | test.cpp:31:14:31:20 | address |
9-
| test.cpp:27:25:27:42 | call to getenv | test.cpp:31:14:31:20 | address |
10-
| test.cpp:38:25:38:30 | call to getenv | test.cpp:42:14:42:20 | address |
11-
| test.cpp:38:25:38:30 | call to getenv | test.cpp:42:14:42:20 | address |
12-
| test.cpp:38:25:38:42 | call to getenv | test.cpp:42:14:42:20 | address |
13-
| test.cpp:38:25:38:42 | call to getenv | test.cpp:42:14:42:20 | address |
14-
| test.cpp:49:25:49:30 | call to getenv | test.cpp:52:14:52:20 | address |
15-
| test.cpp:49:25:49:30 | call to getenv | test.cpp:52:14:52:20 | address |
16-
| test.cpp:49:25:49:30 | call to getenv | test.cpp:56:14:56:20 | address |
17-
| test.cpp:49:25:49:30 | call to getenv | test.cpp:56:14:56:20 | address |
18-
| test.cpp:49:25:49:30 | call to getenv | test.cpp:60:14:60:20 | address |
19-
| test.cpp:49:25:49:30 | call to getenv | test.cpp:60:14:60:20 | address |
20-
| test.cpp:49:25:49:42 | call to getenv | test.cpp:52:14:52:20 | address |
21-
| test.cpp:49:25:49:42 | call to getenv | test.cpp:52:14:52:20 | address |
22-
| test.cpp:49:25:49:42 | call to getenv | test.cpp:56:14:56:20 | address |
23-
| test.cpp:49:25:49:42 | call to getenv | test.cpp:56:14:56:20 | address |
24-
| test.cpp:49:25:49:42 | call to getenv | test.cpp:60:14:60:20 | address |
25-
| test.cpp:49:25:49:42 | call to getenv | test.cpp:60:14:60:20 | address |
26-
subpaths
2+
| test.cpp:16:25:16:42 | call to getenv indirection | test.cpp:20:14:20:20 | address indirection |
3+
| test.cpp:27:25:27:42 | call to getenv indirection | test.cpp:31:14:31:20 | address indirection |
4+
| test.cpp:38:25:38:42 | call to getenv indirection | test.cpp:42:14:42:20 | address indirection |
5+
| test.cpp:49:25:49:42 | call to getenv indirection | test.cpp:52:14:52:20 | address indirection |
6+
| test.cpp:49:25:49:42 | call to getenv indirection | test.cpp:56:14:56:20 | address indirection |
7+
| test.cpp:49:25:49:42 | call to getenv indirection | test.cpp:60:14:60:20 | address indirection |
278
nodes
28-
| test.cpp:16:25:16:30 | call to getenv | semmle.label | call to getenv |
29-
| test.cpp:16:25:16:42 | call to getenv | semmle.label | call to getenv |
30-
| test.cpp:20:14:20:20 | address | semmle.label | address |
31-
| test.cpp:20:14:20:20 | address | semmle.label | address |
32-
| test.cpp:27:25:27:30 | call to getenv | semmle.label | call to getenv |
33-
| test.cpp:27:25:27:42 | call to getenv | semmle.label | call to getenv |
34-
| test.cpp:31:14:31:20 | address | semmle.label | address |
35-
| test.cpp:31:14:31:20 | address | semmle.label | address |
36-
| test.cpp:38:25:38:30 | call to getenv | semmle.label | call to getenv |
37-
| test.cpp:38:25:38:42 | call to getenv | semmle.label | call to getenv |
38-
| test.cpp:42:14:42:20 | address | semmle.label | address |
39-
| test.cpp:42:14:42:20 | address | semmle.label | address |
40-
| test.cpp:49:25:49:30 | call to getenv | semmle.label | call to getenv |
41-
| test.cpp:49:25:49:42 | call to getenv | semmle.label | call to getenv |
42-
| test.cpp:52:14:52:20 | address | semmle.label | address |
43-
| test.cpp:52:14:52:20 | address | semmle.label | address |
44-
| test.cpp:56:14:56:20 | address | semmle.label | address |
45-
| test.cpp:56:14:56:20 | address | semmle.label | address |
46-
| test.cpp:60:14:60:20 | address | semmle.label | address |
47-
| test.cpp:60:14:60:20 | address | semmle.label | address |
9+
| test.cpp:16:25:16:42 | call to getenv indirection | semmle.label | call to getenv indirection |
10+
| test.cpp:20:14:20:20 | address indirection | semmle.label | address indirection |
11+
| test.cpp:27:25:27:42 | call to getenv indirection | semmle.label | call to getenv indirection |
12+
| test.cpp:31:14:31:20 | address indirection | semmle.label | address indirection |
13+
| test.cpp:38:25:38:42 | call to getenv indirection | semmle.label | call to getenv indirection |
14+
| test.cpp:42:14:42:20 | address indirection | semmle.label | address indirection |
15+
| test.cpp:49:25:49:42 | call to getenv indirection | semmle.label | call to getenv indirection |
16+
| test.cpp:52:14:52:20 | address indirection | semmle.label | address indirection |
17+
| test.cpp:56:14:56:20 | address indirection | semmle.label | address indirection |
18+
| test.cpp:60:14:60:20 | address indirection | semmle.label | address indirection |
19+
subpaths
4820
#select
49-
| test.cpp:20:7:20:12 | call to strcmp | test.cpp:16:25:16:30 | call to getenv | test.cpp:20:14:20:20 | address | Untrusted input $@ might be vulnerable to a spoofing attack. | test.cpp:16:25:16:30 | call to getenv | call to getenv |
50-
| test.cpp:31:7:31:12 | call to strcmp | test.cpp:27:25:27:30 | call to getenv | test.cpp:31:14:31:20 | address | Untrusted input $@ might be vulnerable to a spoofing attack. | test.cpp:27:25:27:30 | call to getenv | call to getenv |
51-
| test.cpp:42:7:42:12 | call to strcmp | test.cpp:38:25:38:30 | call to getenv | test.cpp:42:14:42:20 | address | Untrusted input $@ might be vulnerable to a spoofing attack. | test.cpp:38:25:38:30 | call to getenv | call to getenv |
52-
| test.cpp:52:7:52:12 | call to strcmp | test.cpp:49:25:49:30 | call to getenv | test.cpp:52:14:52:20 | address | Untrusted input $@ might be vulnerable to a spoofing attack. | test.cpp:49:25:49:30 | call to getenv | call to getenv |
53-
| test.cpp:56:7:56:12 | call to strcmp | test.cpp:49:25:49:30 | call to getenv | test.cpp:56:14:56:20 | address | Untrusted input $@ might be vulnerable to a spoofing attack. | test.cpp:49:25:49:30 | call to getenv | call to getenv |
54-
| test.cpp:60:7:60:12 | call to strcmp | test.cpp:49:25:49:30 | call to getenv | test.cpp:60:14:60:20 | address | Untrusted input $@ might be vulnerable to a spoofing attack. | test.cpp:49:25:49:30 | call to getenv | call to getenv |
21+
| test.cpp:20:7:20:12 | call to strcmp | test.cpp:16:25:16:42 | call to getenv indirection | test.cpp:20:14:20:20 | address indirection | Untrusted input $@ might be vulnerable to a spoofing attack. | test.cpp:16:25:16:42 | call to getenv indirection | an environment variable |
22+
| test.cpp:31:7:31:12 | call to strcmp | test.cpp:27:25:27:42 | call to getenv indirection | test.cpp:31:14:31:20 | address indirection | Untrusted input $@ might be vulnerable to a spoofing attack. | test.cpp:27:25:27:42 | call to getenv indirection | an environment variable |
23+
| test.cpp:42:7:42:12 | call to strcmp | test.cpp:38:25:38:42 | call to getenv indirection | test.cpp:42:14:42:20 | address indirection | Untrusted input $@ might be vulnerable to a spoofing attack. | test.cpp:38:25:38:42 | call to getenv indirection | an environment variable |
24+
| test.cpp:52:7:52:12 | call to strcmp | test.cpp:49:25:49:42 | call to getenv indirection | test.cpp:52:14:52:20 | address indirection | Untrusted input $@ might be vulnerable to a spoofing attack. | test.cpp:49:25:49:42 | call to getenv indirection | an environment variable |
25+
| test.cpp:56:7:56:12 | call to strcmp | test.cpp:49:25:49:42 | call to getenv indirection | test.cpp:56:14:56:20 | address indirection | Untrusted input $@ might be vulnerable to a spoofing attack. | test.cpp:49:25:49:42 | call to getenv indirection | an environment variable |
26+
| test.cpp:60:7:60:12 | call to strcmp | test.cpp:49:25:49:42 | call to getenv indirection | test.cpp:60:14:60:20 | address indirection | Untrusted input $@ might be vulnerable to a spoofing attack. | test.cpp:49:25:49:42 | call to getenv indirection | an environment variable |

0 commit comments

Comments
 (0)