Skip to content

Commit 5a7cb8f

Browse files
committed
C++: Fix duplication on reference dereference expressions.
1 parent 7998731 commit 5a7cb8f

File tree

3 files changed

+20
-6
lines changed

3 files changed

+20
-6
lines changed

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1273,12 +1273,27 @@ abstract private class IndirectExprNodeBase extends Node {
12731273
}
12741274
}
12751275

1276+
bindingset[e, indirectionIndex]
1277+
private predicate adjustForReference(
1278+
Expr e, int indirectionIndex, Expr conv, int adjustedIndirectionIndex
1279+
) {
1280+
conv.(ReferenceDereferenceExpr).getExpr() = e and
1281+
adjustedIndirectionIndex = indirectionIndex - 1
1282+
or
1283+
not conv instanceof ReferenceDereferenceExpr and
1284+
conv = e and
1285+
adjustedIndirectionIndex = indirectionIndex
1286+
}
1287+
12761288
private class IndirectOperandIndirectExprNode extends IndirectExprNodeBase instanceof IndirectOperand
12771289
{
12781290
IndirectOperandIndirectExprNode() {
12791291
exists(Expr e, int n, int indirectionIndex |
12801292
indirectExprNodeShouldBeIndirectOperand(this, e, n, indirectionIndex) and
1281-
not indirectExprNodeShouldBeIndirectOperand(_, e, n + 1, indirectionIndex)
1293+
not exists(Expr conv, int adjustedIndirectionIndex |
1294+
adjustForReference(e, indirectionIndex, conv, adjustedIndirectionIndex) and
1295+
indirectExprNodeShouldBeIndirectOperand(_, conv, n + 1, adjustedIndirectionIndex)
1296+
)
12821297
)
12831298
}
12841299

@@ -1292,7 +1307,10 @@ private class IndirectInstructionIndirectExprNode extends IndirectExprNodeBase i
12921307
IndirectInstructionIndirectExprNode() {
12931308
exists(Expr e, int n, int indirectionIndex |
12941309
indirectExprNodeShouldBeIndirectInstruction(this, e, n, indirectionIndex) and
1295-
not indirectExprNodeShouldBeIndirectInstruction(_, e, n + 1, indirectionIndex)
1310+
not exists(Expr conv, int adjustedIndirectionIndex |
1311+
adjustForReference(e, indirectionIndex, conv, adjustedIndirectionIndex) and
1312+
not indirectExprNodeShouldBeIndirectInstruction(_, conv, n + 1, adjustedIndirectionIndex)
1313+
)
12961314
)
12971315
}
12981316

cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,6 @@ irFlow
282282
| test.cpp:832:21:832:26 | call to source | test.cpp:836:10:836:22 | global_direct |
283283
| test.cpp:842:11:842:16 | call to source | test.cpp:844:8:844:8 | y |
284284
| test.cpp:846:13:846:27 | call to indirect_source indirection | test.cpp:848:17:848:25 | rpx indirection |
285-
| test.cpp:846:13:846:27 | call to indirect_source indirection | test.cpp:848:23:848:25 | (reference dereference) indirection |
286285
| true_upon_entry.cpp:9:11:9:16 | call to source | true_upon_entry.cpp:13:8:13:8 | x |
287286
| true_upon_entry.cpp:17:11:17:16 | call to source | true_upon_entry.cpp:21:8:21:8 | x |
288287
| true_upon_entry.cpp:27:9:27:14 | call to source | true_upon_entry.cpp:29:8:29:8 | x |

cpp/ql/test/query-tests/Security/CWE/CWE-114/semmle/UncontrolledProcessOperation/UncontrolledProcessOperation.expected

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ edges
55
| test.cpp:43:18:43:34 | call to getenv indirection | test.cpp:29:30:29:36 | command indirection |
66
| test.cpp:56:12:56:17 | fgets output argument | test.cpp:62:10:62:15 | buffer indirection |
77
| test.cpp:56:12:56:17 | fgets output argument | test.cpp:63:10:63:13 | data indirection |
8-
| test.cpp:56:12:56:17 | fgets output argument | test.cpp:64:10:64:16 | (reference dereference) indirection |
98
| test.cpp:56:12:56:17 | fgets output argument | test.cpp:64:10:64:16 | dataref indirection |
109
| test.cpp:56:12:56:17 | fgets output argument | test.cpp:65:10:65:14 | data2 indirection |
1110
| test.cpp:76:12:76:17 | fgets output argument | test.cpp:78:10:78:15 | buffer indirection |
@@ -22,7 +21,6 @@ nodes
2221
| test.cpp:56:12:56:17 | fgets output argument | semmle.label | fgets output argument |
2322
| test.cpp:62:10:62:15 | buffer indirection | semmle.label | buffer indirection |
2423
| test.cpp:63:10:63:13 | data indirection | semmle.label | data indirection |
25-
| test.cpp:64:10:64:16 | (reference dereference) indirection | semmle.label | (reference dereference) indirection |
2624
| test.cpp:64:10:64:16 | dataref indirection | semmle.label | dataref indirection |
2725
| test.cpp:65:10:65:14 | data2 indirection | semmle.label | data2 indirection |
2826
| test.cpp:76:12:76:17 | fgets output argument | semmle.label | fgets output argument |
@@ -39,7 +37,6 @@ subpaths
3937
| test.cpp:31:10:31:16 | command indirection | test.cpp:43:18:43:34 | call to getenv indirection | test.cpp:31:10:31:16 | command indirection | The value of this argument may come from $@ and is being passed to system. | test.cpp:43:18:43:34 | call to getenv indirection | an environment variable |
4038
| test.cpp:62:10:62:15 | buffer indirection | test.cpp:56:12:56:17 | fgets output argument | test.cpp:62:10:62:15 | buffer indirection | The value of this argument may come from $@ and is being passed to system. | test.cpp:56:12:56:17 | fgets output argument | string read by fgets |
4139
| test.cpp:63:10:63:13 | data indirection | test.cpp:56:12:56:17 | fgets output argument | test.cpp:63:10:63:13 | data indirection | The value of this argument may come from $@ and is being passed to system. | test.cpp:56:12:56:17 | fgets output argument | string read by fgets |
42-
| test.cpp:64:10:64:16 | (reference dereference) indirection | test.cpp:56:12:56:17 | fgets output argument | test.cpp:64:10:64:16 | (reference dereference) indirection | The value of this argument may come from $@ and is being passed to system. | test.cpp:56:12:56:17 | fgets output argument | string read by fgets |
4340
| test.cpp:64:10:64:16 | dataref indirection | test.cpp:56:12:56:17 | fgets output argument | test.cpp:64:10:64:16 | dataref indirection | The value of this argument may come from $@ and is being passed to system. | test.cpp:56:12:56:17 | fgets output argument | string read by fgets |
4441
| test.cpp:65:10:65:14 | data2 indirection | test.cpp:56:12:56:17 | fgets output argument | test.cpp:65:10:65:14 | data2 indirection | The value of this argument may come from $@ and is being passed to system. | test.cpp:56:12:56:17 | fgets output argument | string read by fgets |
4542
| test.cpp:78:10:78:15 | buffer indirection | test.cpp:76:12:76:17 | fgets output argument | test.cpp:78:10:78:15 | buffer indirection | The value of this argument may come from $@ and is being passed to system. | test.cpp:76:12:76:17 | fgets output argument | string read by fgets |

0 commit comments

Comments
 (0)