Skip to content

Commit 39d44ad

Browse files
MathiasVPMathiasVP
authored
Merge pull request #11896 from jketema/spurious-default-taint
C++: Fix spurious results in default taint tracking
2 parents 2aace0d + a892ae8 commit 39d44ad

5 files changed

Lines changed: 12 additions & 26 deletions

File tree

‎cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DefaultTaintTrackingImpl.qll‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -168,16 +168,18 @@ private predicate hasUpperBoundsCheck(Variable var) {
168168
private predicate nodeIsBarrierEqualityCandidate(
169169
DataFlow::Node node, Operand access, Variable checkedVar
170170
) {
171-
readsVariable(node.asInstruction(), checkedVar) and
172-
any(IRGuardCondition guard).ensuresEq(access, _, _, node.asInstruction().getBlock(), true)
171+
exists(Instruction instr | instr = node.asOperand().getDef() |
172+
readsVariable(instr, checkedVar) and
173+
any(IRGuardCondition guard).ensuresEq(access, _, _, instr.getBlock(), true)
174+
)
173175
}
174176

175177
cached
176178
private module Cached {
177179
cached
178180
predicate nodeIsBarrier(DataFlow::Node node) {
179-
exists(Variable checkedVar |
180-
node.asExpr().(VariableAccess).getTarget() = checkedVar and
181+
exists(Variable checkedVar, Instruction instr | instr = node.asOperand().getDef() |
182+
readsVariable(instr, checkedVar) and
181183
hasUpperBoundsCheck(checkedVar)
182184
)
183185
or

‎cpp/ql/src/Security/CWE/CWE-190/TaintedAllocationSize.ql‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,10 @@ predicate hasUpperBoundsCheck(Variable var) {
4646
}
4747

4848
predicate nodeIsBarrierEqualityCandidate(DataFlow::Node node, Operand access, Variable checkedVar) {
49-
readsVariable(node.asInstruction(), checkedVar) and
50-
any(IRGuardCondition guard).ensuresEq(access, _, _, node.asInstruction().getBlock(), true)
49+
exists(Instruction instr | instr = node.asOperand().getDef() |
50+
readsVariable(instr, checkedVar) and
51+
any(IRGuardCondition guard).ensuresEq(access, _, _, instr.getBlock(), true)
52+
)
5153
}
5254

5355
predicate isFlowSource(FlowSource source, string sourceType) { sourceType = source.getSourceType() }
@@ -79,8 +81,8 @@ class TaintedAllocationSizeConfiguration extends TaintTracking::Configuration {
7981
e = any(PointerDiffExpr diff).getAnOperand()
8082
)
8183
or
82-
exists(Variable checkedVar |
83-
node.asExpr().(VariableAccess).getTarget() = checkedVar and
84+
exists(Variable checkedVar, Instruction instr | instr = node.asOperand().getDef() |
85+
readsVariable(instr, checkedVar) and
8486
hasUpperBoundsCheck(checkedVar)
8587
)
8688
or

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,6 @@ edges
4242
| test.cpp:259:20:259:33 | call to getenv indirection | test.cpp:263:11:263:29 | ... * ... |
4343
| test.cpp:289:17:289:20 | get_size output argument | test.cpp:291:11:291:28 | ... * ... |
4444
| test.cpp:305:18:305:21 | get_size output argument | test.cpp:308:10:308:27 | ... * ... |
45-
| test.cpp:338:19:338:24 | Call | test.cpp:342:25:342:43 | ... * ... |
46-
| test.cpp:338:19:338:32 | call to getenv indirection | test.cpp:342:25:342:43 | ... * ... |
4745
| test.cpp:353:18:353:23 | Call | test.cpp:355:35:355:38 | size |
4846
| test.cpp:353:18:353:23 | Call | test.cpp:356:35:356:38 | size |
4947
| test.cpp:353:18:353:31 | call to getenv indirection | test.cpp:355:35:355:38 | size |
@@ -88,9 +86,6 @@ nodes
8886
| test.cpp:291:11:291:28 | ... * ... | semmle.label | ... * ... |
8987
| test.cpp:305:18:305:21 | get_size output argument | semmle.label | get_size output argument |
9088
| test.cpp:308:10:308:27 | ... * ... | semmle.label | ... * ... |
91-
| test.cpp:338:19:338:24 | Call | semmle.label | Call |
92-
| test.cpp:338:19:338:32 | call to getenv indirection | semmle.label | call to getenv indirection |
93-
| test.cpp:342:25:342:43 | ... * ... | semmle.label | ... * ... |
9489
| test.cpp:353:18:353:23 | Call | semmle.label | Call |
9590
| test.cpp:353:18:353:31 | call to getenv indirection | semmle.label | call to getenv indirection |
9691
| test.cpp:355:35:355:38 | size | semmle.label | size |
@@ -135,8 +130,6 @@ subpaths
135130
| test.cpp:291:4:291:9 | call to malloc | test.cpp:251:18:251:31 | call to getenv indirection | test.cpp:291:11:291:28 | ... * ... | This allocation size is derived from $@ and might overflow. | test.cpp:251:18:251:31 | call to getenv indirection | user input (an environment variable) |
136131
| test.cpp:308:3:308:8 | call to malloc | test.cpp:251:18:251:23 | Call | test.cpp:308:10:308:27 | ... * ... | This allocation size is derived from $@ and might overflow. | test.cpp:251:18:251:23 | Call | user input (an environment variable) |
137132
| test.cpp:308:3:308:8 | call to malloc | test.cpp:251:18:251:31 | call to getenv indirection | test.cpp:308:10:308:27 | ... * ... | This allocation size is derived from $@ and might overflow. | test.cpp:251:18:251:31 | call to getenv indirection | user input (an environment variable) |
138-
| test.cpp:342:18:342:23 | call to malloc | test.cpp:338:19:338:24 | Call | test.cpp:342:25:342:43 | ... * ... | This allocation size is derived from $@ and might overflow. | test.cpp:338:19:338:24 | Call | user input (an environment variable) |
139-
| test.cpp:342:18:342:23 | call to malloc | test.cpp:338:19:338:32 | call to getenv indirection | test.cpp:342:25:342:43 | ... * ... | This allocation size is derived from $@ and might overflow. | test.cpp:338:19:338:32 | call to getenv indirection | user input (an environment variable) |
140133
| test.cpp:355:25:355:33 | call to MyMalloc1 | test.cpp:353:18:353:23 | Call | test.cpp:355:35:355:38 | size | This allocation size is derived from $@ and might overflow. | test.cpp:353:18:353:23 | Call | user input (an environment variable) |
141134
| test.cpp:355:25:355:33 | call to MyMalloc1 | test.cpp:353:18:353:31 | call to getenv indirection | test.cpp:355:35:355:38 | size | This allocation size is derived from $@ and might overflow. | test.cpp:353:18:353:31 | call to getenv indirection | user input (an environment variable) |
142135
| test.cpp:356:25:356:33 | call to MyMalloc2 | test.cpp:353:18:353:23 | Call | test.cpp:356:35:356:38 | size | This allocation size is derived from $@ and might overflow. | test.cpp:353:18:353:23 | Call | user input (an environment variable) |

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,6 @@ edges
2323
| test.c:51:17:51:20 | argv | test.c:54:7:54:10 | len3 |
2424
| test.c:51:17:51:20 | argv | test.c:54:7:54:10 | len3 |
2525
| test.c:51:17:51:20 | argv | test.c:54:7:54:10 | len3 |
26-
| test.c:99:30:99:33 | argv | test.c:102:17:102:31 | maxConnections3 |
27-
| test.c:99:30:99:33 | argv | test.c:102:17:102:31 | maxConnections3 |
28-
| test.c:99:30:99:33 | argv | test.c:102:17:102:31 | maxConnections3 |
29-
| test.c:99:30:99:33 | argv | test.c:102:17:102:31 | maxConnections3 |
3026
subpaths
3127
nodes
3228
| test2.cpp:12:21:12:21 | v | semmle.label | v |
@@ -55,10 +51,6 @@ nodes
5551
| test.c:51:17:51:20 | argv | semmle.label | argv |
5652
| test.c:54:7:54:10 | len3 | semmle.label | len3 |
5753
| test.c:54:7:54:10 | len3 | semmle.label | len3 |
58-
| test.c:99:30:99:33 | argv | semmle.label | argv |
59-
| test.c:99:30:99:33 | argv | semmle.label | argv |
60-
| test.c:102:17:102:31 | maxConnections3 | semmle.label | maxConnections3 |
61-
| test.c:102:17:102:31 | maxConnections3 | semmle.label | maxConnections3 |
6254
#select
6355
| test2.cpp:14:11:14:11 | v | test2.cpp:25:22:25:23 | & ... | test2.cpp:14:11:14:11 | v | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test2.cpp:25:22:25:23 | & ... | User-provided value |
6456
| test2.cpp:14:11:14:11 | v | test2.cpp:25:22:25:23 | & ... | test2.cpp:14:11:14:11 | v | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test2.cpp:25:22:25:23 | & ... | User-provided value |
@@ -69,5 +61,3 @@ nodes
6961
| test.c:14:15:14:28 | maxConnections | test.c:11:29:11:32 | argv | test.c:14:15:14:28 | maxConnections | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test.c:11:29:11:32 | argv | User-provided value |
7062
| test.c:44:7:44:10 | len2 | test.c:41:17:41:20 | argv | test.c:44:7:44:10 | len2 | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test.c:41:17:41:20 | argv | User-provided value |
7163
| test.c:54:7:54:10 | len3 | test.c:51:17:51:20 | argv | test.c:54:7:54:10 | len3 | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test.c:51:17:51:20 | argv | User-provided value |
72-
| test.c:102:17:102:31 | maxConnections3 | test.c:99:30:99:33 | argv | test.c:102:17:102:31 | maxConnections3 | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test.c:99:30:99:33 | argv | User-provided value |
73-
| test.c:102:17:102:31 | maxConnections3 | test.c:99:30:99:33 | argv | test.c:102:17:102:31 | maxConnections3 | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test.c:99:30:99:33 | argv | User-provided value |

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,3 @@
1414
| test.c:14:15:14:35 | ... * ... | $@ flows an expression which might overflow. | test.c:11:29:11:32 | argv | User-provided value |
1515
| test.c:44:7:44:12 | ... -- | $@ flows an expression which might overflow negatively. | test.c:41:17:41:20 | argv | User-provided value |
1616
| test.c:54:7:54:12 | ... -- | $@ flows an expression which might overflow negatively. | test.c:51:17:51:20 | argv | User-provided value |
17-
| test.c:102:17:102:38 | ... * ... | $@ flows an expression which might overflow. | test.c:99:30:99:33 | argv | User-provided value |

0 commit comments

Comments
 (0)