From 8e005a65bf8a2f76a86f22e554605256b48bb010 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 16 May 2025 20:12:58 +0100 Subject: [PATCH 1/4] C++: Fix missing 'asExpr' for array aggregate literals. --- .../cpp/ir/dataflow/internal/ExprNodes.qll | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ExprNodes.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ExprNodes.qll index 5514bd80eeec..42ab60eced78 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ExprNodes.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ExprNodes.qll @@ -45,6 +45,28 @@ private module Cached { ) } + private Expr getRankedElementExpr(ArrayAggregateLiteral aggr, int rnk) { + result = + rank[rnk + 1](Expr e, int elementIndex, int position | + e = aggr.getElementExpr(elementIndex, position) + | + e order by elementIndex, position + ) + } + + private class LastArrayAggregateStore extends StoreInstruction { + ArrayAggregateLiteral aggr; + + LastArrayAggregateStore() { + exists(int rnk | + this.getSourceValue().getUnconvertedResultExpression() = getRankedElementExpr(aggr, rnk) and + not exists(getRankedElementExpr(aggr, rnk + 1)) + ) + } + + ArrayAggregateLiteral getArrayAggregateLiteral() { result = aggr } + } + private Expr getConvertedResultExpressionImpl0(Instruction instr) { // IR construction inserts an additional cast to a `size_t` on the extent // of a `new[]` expression. The resulting `ConvertInstruction` doesn't have @@ -95,6 +117,16 @@ private module Cached { tco.producesExprResult() and result = asDefinitionImpl0(instr) ) + or + // IR construction breaks an array aggregate literal `{1, 2, 3}` into a + // sequence of `StoreInstruction`s. So there's no instruction `i` for which + // `i.getUnconvertedResultExpression() instanceof ArrayAggregateLiteral`. + // So we map the instruction node corresponding to the last `Store` + // instruction of the sequence to the result of the array aggregate + // literal. This makes sense since this store will immediately flow into + // the indirect node representing the array. So this node does represent + // the array after it has been fully initialized. + result = instr.(LastArrayAggregateStore).getArrayAggregateLiteral() } private Expr getConvertedResultExpressionImpl(Instruction instr) { From ced1d580dff30c53faaef8f4469b3ef1840a2f56 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 16 May 2025 20:14:10 +0100 Subject: [PATCH 2/4] C++: Accept test changes. --- cpp/ql/test/library-tests/dataflow/asExpr/test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/ql/test/library-tests/dataflow/asExpr/test.cpp b/cpp/ql/test/library-tests/dataflow/asExpr/test.cpp index c81b86aa8ae3..fc9a4e6be082 100644 --- a/cpp/ql/test/library-tests/dataflow/asExpr/test.cpp +++ b/cpp/ql/test/library-tests/dataflow/asExpr/test.cpp @@ -35,6 +35,6 @@ void test_aggregate_literal() { S s5 = {.a = 1, .b = 2}; // $ asExpr=1 asExpr=2 asExpr={...} - int xs[] = {1, 2, 3}; // $ asExpr=1 asExpr=2 asExpr=3 MISSING: asExpr={...} - const int ys[] = {[0] = 4, [1] = 5, [0] = 6}; // $ asExpr=4 asExpr=5 asExpr=6 MISSING: asExpr={...} + int xs[] = {1, 2, 3}; // $ asExpr=1 asExpr=2 asExpr=3 asExpr={...} + const int ys[] = {[0] = 4, [1] = 5, [0] = 6}; // $ asExpr=4 asExpr=5 asExpr=6 asExpr={...} } \ No newline at end of file From 0eb55779fbcd875278312abdbe2886276e81c38f Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 16 May 2025 20:30:21 +0100 Subject: [PATCH 3/4] C++: Add change note. --- .../lib/change-notes/2025-05-16-array-aggregate-literals.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 cpp/ql/lib/change-notes/2025-05-16-array-aggregate-literals.md diff --git a/cpp/ql/lib/change-notes/2025-05-16-array-aggregate-literals.md b/cpp/ql/lib/change-notes/2025-05-16-array-aggregate-literals.md new file mode 100644 index 000000000000..a1aec0a695a7 --- /dev/null +++ b/cpp/ql/lib/change-notes/2025-05-16-array-aggregate-literals.md @@ -0,0 +1,4 @@ +--- +category: fix +--- +* Fixed a problem where `asExpr()` on `DataFlow::Node` would never return `ArrayAggregateLiteral`s. \ No newline at end of file From ff11aaf2bb8f6d77c1df811086237a4f845245de Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 16 May 2025 21:01:55 +0100 Subject: [PATCH 4/4] C++: Accept query test 'toString' improvements. --- .../semmle/consts/NonConstantFormat.expected | 16 ++++++++-------- .../CWE/CWE-319/UseOfHttp/UseOfHttp.expected | 6 +++--- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/consts/NonConstantFormat.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/consts/NonConstantFormat.expected index 421d12dabd31..c2a952774ff0 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/consts/NonConstantFormat.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/consts/NonConstantFormat.expected @@ -2,8 +2,8 @@ edges | consts.cpp:24:7:24:9 | **gv1 | consts.cpp:25:2:25:4 | *a | provenance | | | consts.cpp:24:7:24:9 | **gv1 | consts.cpp:30:9:30:14 | *access to array | provenance | | | consts.cpp:24:7:24:9 | **gv1 | consts.cpp:123:2:123:12 | *... = ... | provenance | | -| consts.cpp:25:2:25:4 | *a | consts.cpp:26:2:26:4 | *b | provenance | | -| consts.cpp:26:2:26:4 | *b | consts.cpp:24:7:24:9 | **gv1 | provenance | | +| consts.cpp:25:2:25:4 | *a | consts.cpp:26:2:26:4 | *{...} | provenance | | +| consts.cpp:26:2:26:4 | *{...} | consts.cpp:24:7:24:9 | **gv1 | provenance | | | consts.cpp:29:7:29:25 | **nonConstFuncToArray | consts.cpp:126:9:126:30 | *call to nonConstFuncToArray | provenance | | | consts.cpp:30:9:30:14 | *access to array | consts.cpp:29:7:29:25 | **nonConstFuncToArray | provenance | | | consts.cpp:85:7:85:8 | gets output argument | consts.cpp:86:9:86:10 | *v1 | provenance | | @@ -14,7 +14,7 @@ edges | consts.cpp:85:7:85:8 | gets output argument | consts.cpp:129:19:129:20 | *v1 | provenance | | | consts.cpp:85:7:85:8 | gets output argument | consts.cpp:135:9:135:11 | *v10 | provenance | TaintFunction | | consts.cpp:90:2:90:14 | *... = ... | consts.cpp:91:9:91:10 | *v2 | provenance | | -| consts.cpp:90:2:90:14 | *... = ... | consts.cpp:115:21:115:22 | *v2 | provenance | | +| consts.cpp:90:2:90:14 | *... = ... | consts.cpp:115:21:115:22 | *{...} | provenance | | | consts.cpp:90:7:90:10 | *call to gets | consts.cpp:90:2:90:14 | *... = ... | provenance | | | consts.cpp:90:12:90:13 | gets output argument | consts.cpp:94:13:94:14 | *v1 | provenance | | | consts.cpp:90:12:90:13 | gets output argument | consts.cpp:99:2:99:8 | *... = ... | provenance | | @@ -28,9 +28,9 @@ edges | consts.cpp:106:13:106:19 | *call to varFunc | consts.cpp:107:9:107:10 | *v5 | provenance | | | consts.cpp:111:2:111:15 | *... = ... | consts.cpp:112:9:112:10 | *v6 | provenance | | | consts.cpp:111:7:111:13 | *call to varFunc | consts.cpp:111:2:111:15 | *... = ... | provenance | | -| consts.cpp:115:17:115:18 | *v1 | consts.cpp:115:21:115:22 | *v2 | provenance | | -| consts.cpp:115:21:115:22 | *v2 | consts.cpp:116:9:116:13 | *access to array | provenance | | -| consts.cpp:115:21:115:22 | *v2 | consts.cpp:120:2:120:11 | *... = ... | provenance | | +| consts.cpp:115:17:115:18 | *v1 | consts.cpp:115:21:115:22 | *{...} | provenance | | +| consts.cpp:115:21:115:22 | *{...} | consts.cpp:116:9:116:13 | *access to array | provenance | | +| consts.cpp:115:21:115:22 | *{...} | consts.cpp:120:2:120:11 | *... = ... | provenance | | | consts.cpp:120:2:120:11 | *... = ... | consts.cpp:121:9:121:10 | *v8 | provenance | | | consts.cpp:123:2:123:12 | *... = ... | consts.cpp:24:7:24:9 | **gv1 | provenance | | | consts.cpp:129:19:129:20 | *v1 | consts.cpp:130:9:130:10 | *v9 | provenance | | @@ -39,7 +39,7 @@ edges nodes | consts.cpp:24:7:24:9 | **gv1 | semmle.label | **gv1 | | consts.cpp:25:2:25:4 | *a | semmle.label | *a | -| consts.cpp:26:2:26:4 | *b | semmle.label | *b | +| consts.cpp:26:2:26:4 | *{...} | semmle.label | *{...} | | consts.cpp:29:7:29:25 | **nonConstFuncToArray | semmle.label | **nonConstFuncToArray | | consts.cpp:30:9:30:14 | *access to array | semmle.label | *access to array | | consts.cpp:85:7:85:8 | gets output argument | semmle.label | gets output argument | @@ -60,7 +60,7 @@ nodes | consts.cpp:111:7:111:13 | *call to varFunc | semmle.label | *call to varFunc | | consts.cpp:112:9:112:10 | *v6 | semmle.label | *v6 | | consts.cpp:115:17:115:18 | *v1 | semmle.label | *v1 | -| consts.cpp:115:21:115:22 | *v2 | semmle.label | *v2 | +| consts.cpp:115:21:115:22 | *{...} | semmle.label | *{...} | | consts.cpp:116:9:116:13 | *access to array | semmle.label | *access to array | | consts.cpp:120:2:120:11 | *... = ... | semmle.label | *... = ... | | consts.cpp:121:9:121:10 | *v8 | semmle.label | *v8 | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-319/UseOfHttp/UseOfHttp.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-319/UseOfHttp/UseOfHttp.expected index a978b9edd7d2..971cdb4f3ff3 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-319/UseOfHttp/UseOfHttp.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-319/UseOfHttp/UseOfHttp.expected @@ -6,8 +6,8 @@ edges | test.cpp:28:10:28:29 | *http://example.com | test.cpp:11:26:11:28 | *url | provenance | | | test.cpp:35:23:35:42 | *http://example.com | test.cpp:35:23:35:42 | *http://example.com | provenance | | | test.cpp:35:23:35:42 | *http://example.com | test.cpp:39:11:39:15 | *url_l | provenance | | -| test.cpp:36:26:36:45 | *http://example.com | test.cpp:36:26:36:45 | *http://example.com | provenance | | -| test.cpp:36:26:36:45 | *http://example.com | test.cpp:40:11:40:17 | *access to array | provenance | | +| test.cpp:36:26:36:45 | *http://example.com | test.cpp:36:26:36:45 | *{...} | provenance | | +| test.cpp:36:26:36:45 | *{...} | test.cpp:40:11:40:17 | *access to array | provenance | | | test.cpp:38:11:38:15 | *url_g | test.cpp:11:26:11:28 | *url | provenance | | | test.cpp:39:11:39:15 | *url_l | test.cpp:11:26:11:28 | *url | provenance | | | test.cpp:40:11:40:17 | *access to array | test.cpp:11:26:11:28 | *url | provenance | | @@ -29,7 +29,7 @@ nodes | test.cpp:35:23:35:42 | *http://example.com | semmle.label | *http://example.com | | test.cpp:35:23:35:42 | *http://example.com | semmle.label | *http://example.com | | test.cpp:36:26:36:45 | *http://example.com | semmle.label | *http://example.com | -| test.cpp:36:26:36:45 | *http://example.com | semmle.label | *http://example.com | +| test.cpp:36:26:36:45 | *{...} | semmle.label | *{...} | | test.cpp:38:11:38:15 | *url_g | semmle.label | *url_g | | test.cpp:39:11:39:15 | *url_l | semmle.label | *url_l | | test.cpp:40:11:40:17 | *access to array | semmle.label | *access to array |