Merge pull request #18531 from MathiasVP/remove-conflation-from-out-nodes

C++: Remove pointer/pointee conflation from parameter nodes

Author: MathiasVP

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

@@ -382,7 +382,7 @@ private class SideEffectArgumentNode extends ArgumentNode, SideEffectOperandNode
     exists(int indirectionIndex |
       pos = TIndirectionPosition(argumentIndex, pragma[only_bind_into](indirectionIndex)) and
       this.getCallInstruction() = dfCall.asCallInstruction() and
-      super.hasAddressOperandAndIndirectionIndex(_, pragma[only_bind_into](indirectionIndex))
+      super.hasAddressOperandAndIndirectionIndex(arg, pragma[only_bind_into](indirectionIndex))
     )
   }
 }

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

@@ -757,9 +757,11 @@ class SsaIteratorNode extends Node, TSsaIteratorNode {
 class SideEffectOperandNode extends Node instanceof IndirectOperand {
   CallInstruction call;
   int argumentIndex;
+  ArgumentOperand arg;
 
   SideEffectOperandNode() {
-    IndirectOperand.super.hasOperandAndIndirectionIndex(call.getArgumentOperand(argumentIndex), _)
+    arg = call.getArgumentOperand(argumentIndex) and
+    IndirectOperand.super.hasOperandAndIndirectionIndex(arg, _)
   }
 
   CallInstruction getCallInstruction() { result = call }

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-409/DecompressionBombs/DecompressionBombs.expected

@@ -33,13 +33,9 @@ edges
 | main.cpp:10:24:10:27 | minizip_test output argument | main.cpp:11:21:11:24 | **argv | provenance |  |
 | main.cpp:10:24:10:27 | minizip_test output argument | main.cpp:12:21:12:24 | **argv | provenance |  |
 | main.cpp:11:21:11:24 | **argv | main.cpp:11:21:11:24 | zlib_test output argument | provenance |  |
-| main.cpp:11:21:11:24 | **argv | main.cpp:11:21:11:24 | zlib_test output argument | provenance |  |
 | main.cpp:11:21:11:24 | **argv | zlibTest.cpp:80:33:80:36 | **argv | provenance |  |
 | main.cpp:11:21:11:24 | zlib_test output argument | main.cpp:12:21:12:24 | **argv | provenance |  |
-| main.cpp:11:21:11:24 | zlib_test output argument | main.cpp:12:21:12:24 | *argv | provenance |  |
 | main.cpp:12:21:12:24 | **argv | zstdTest.cpp:26:39:26:42 | **argv | provenance |  |
-| main.cpp:12:21:12:24 | *argv | zstdTest.cpp:26:39:26:42 | **argv | provenance |  |
-| main.cpp:12:21:12:24 | *argv | zstdTest.cpp:26:39:26:42 | *argv | provenance |  |
 | minizipTest.cpp:12:42:12:45 | **argv | minizipTest.cpp:12:42:12:45 | **argv | provenance |  |
 | minizipTest.cpp:12:42:12:45 | **argv | minizipTest.cpp:17:52:17:67 | *access to array | provenance |  |
 | minizipTest.cpp:12:42:12:45 | **argv | minizipTest.cpp:24:41:24:47 | *access to array | provenance |  |
@@ -107,7 +103,6 @@ edges
 | zlibTest.cpp:85:19:85:25 | InflateString output argument | zlibTest.cpp:80:33:80:36 | **argv | provenance |  |
 | zlibTest.cpp:85:19:85:25 | InflateString output argument | zlibTest.cpp:80:33:80:36 | **argv [Return] | provenance |  |
 | zstdTest.cpp:26:39:26:42 | **argv | zstdTest.cpp:27:35:27:41 | *access to array | provenance |  |
-| zstdTest.cpp:26:39:26:42 | *argv | zstdTest.cpp:27:35:27:41 | *access to array | provenance |  |
 | zstdTest.cpp:27:23:27:33 | call to fopen_orDie | zstdTest.cpp:27:23:27:33 | call to fopen_orDie | provenance |  |
 | zstdTest.cpp:27:23:27:33 | call to fopen_orDie | zstdTest.cpp:35:52:35:54 | fin | provenance |  |
 | zstdTest.cpp:27:35:27:41 | *access to array | zstdTest.cpp:27:23:27:33 | call to fopen_orDie | provenance | Config |
@@ -147,9 +142,7 @@ nodes
 | main.cpp:10:24:10:27 | minizip_test output argument | semmle.label | minizip_test output argument |
 | main.cpp:11:21:11:24 | **argv | semmle.label | **argv |
 | main.cpp:11:21:11:24 | zlib_test output argument | semmle.label | zlib_test output argument |
-| main.cpp:11:21:11:24 | zlib_test output argument | semmle.label | zlib_test output argument |
 | main.cpp:12:21:12:24 | **argv | semmle.label | **argv |
-| main.cpp:12:21:12:24 | *argv | semmle.label | *argv |
 | minizipTest.cpp:12:42:12:45 | **argv | semmle.label | **argv |
 | minizipTest.cpp:12:42:12:45 | **argv | semmle.label | **argv |
 | minizipTest.cpp:17:52:17:67 | *access to array | semmle.label | *access to array |
@@ -199,7 +192,6 @@ nodes
 | zlibTest.cpp:85:19:85:25 | *access to array | semmle.label | *access to array |
 | zlibTest.cpp:85:19:85:25 | InflateString output argument | semmle.label | InflateString output argument |
 | zstdTest.cpp:26:39:26:42 | **argv | semmle.label | **argv |
-| zstdTest.cpp:26:39:26:42 | *argv | semmle.label | *argv |
 | zstdTest.cpp:27:23:27:33 | call to fopen_orDie | semmle.label | call to fopen_orDie |
 | zstdTest.cpp:27:23:27:33 | call to fopen_orDie | semmle.label | call to fopen_orDie |
 | zstdTest.cpp:27:35:27:41 | *access to array | semmle.label | *access to array |
@@ -217,7 +209,6 @@ subpaths
 | main.cpp:10:24:10:27 | **argv | minizipTest.cpp:12:42:12:45 | **argv | minizipTest.cpp:12:42:12:45 | **argv | main.cpp:10:24:10:27 | minizip_test output argument |
 | main.cpp:11:21:11:24 | **argv | zlibTest.cpp:80:33:80:36 | **argv | zlibTest.cpp:80:33:80:36 | **argv | main.cpp:11:21:11:24 | zlib_test output argument |
 | main.cpp:11:21:11:24 | **argv | zlibTest.cpp:80:33:80:36 | **argv | zlibTest.cpp:80:33:80:36 | **argv [Return] | main.cpp:11:21:11:24 | zlib_test output argument |
-| main.cpp:11:21:11:24 | **argv | zlibTest.cpp:80:33:80:36 | **argv | zlibTest.cpp:80:33:80:36 | **argv [Return] | main.cpp:11:21:11:24 | zlib_test output argument |
 | zlibTest.cpp:81:19:81:25 | *access to array | zlibTest.cpp:47:26:47:33 | *fileName | zlibTest.cpp:47:26:47:33 | *fileName | zlibTest.cpp:81:19:81:25 | UnsafeGzfread output argument |
 | zlibTest.cpp:82:18:82:24 | *access to array | zlibTest.cpp:57:25:57:32 | *fileName | zlibTest.cpp:57:25:57:32 | *fileName | zlibTest.cpp:82:18:82:24 | UnsafeGzgets output argument |
 | zlibTest.cpp:83:19:83:25 | *access to array | zlibTest.cpp:16:26:16:30 | *input | zlibTest.cpp:16:26:16:30 | *input | zlibTest.cpp:83:19:83:25 | UnsafeInflate output argument |

cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected

@@ -185,6 +185,8 @@ postWithInFlow
 | test.cpp:1138:5:1138:8 | data [inner post update] | PostUpdateNode should not be the target of local flow. |
 | test.cpp:1139:3:1139:7 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
 | test.cpp:1139:4:1139:7 | data [inner post update] | PostUpdateNode should not be the target of local flow. |
+| test.cpp:1153:5:1153:6 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
+| test.cpp:1153:6:1153:6 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
 viableImplInCallContextTooLarge
 uniqueParameterNodeAtPosition
 uniqueParameterNodePosition

cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp

@@ -1138,4 +1138,19 @@ void test_uncertain_array(int n1, int n2) {
   *(data + 1) = source();
   *data = 0;
   sink(*(data + 1)); // $ ast=1138:17 ast=1137:7 ir
+}
+
+namespace conflation_regression {
+
+  char* source(int);
+
+  void read_deref_deref(char **l) { // $ ast-def=l ir-def=*l ir-def=**l
+    sink(**l); // Clean. Only *l is tainted
+  }
+
+  void f(char ** p) // $ ast-def=p ir-def=*p ir-def=**p
+  {
+    *p = source(0);
+    read_deref_deref(p);
+  }
 }

cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/UnboundedWrite.expected

@@ -16,8 +16,6 @@ edges
 | main.cpp:8:34:8:37 | **argv | main.cpp:8:34:8:37 | test_buffer_overrun_main output argument | provenance |  |
 | main.cpp:8:34:8:37 | **argv | test_buffer_overrun.cpp:32:46:32:49 | **argv | provenance |  |
 | main.cpp:8:34:8:37 | *argv | main.cpp:8:34:8:37 | test_buffer_overrun_main output argument | provenance |  |
-| main.cpp:8:34:8:37 | *argv | main.cpp:8:34:8:37 | test_buffer_overrun_main output argument | provenance |  |
-| main.cpp:8:34:8:37 | *argv | test_buffer_overrun.cpp:32:46:32:49 | **argv | provenance |  |
 | main.cpp:8:34:8:37 | *argv | test_buffer_overrun.cpp:32:46:32:49 | *argv | provenance |  |
 | main.cpp:8:34:8:37 | test_buffer_overrun_main output argument | main.cpp:9:29:9:32 | **argv | provenance |  |
 | main.cpp:8:34:8:37 | test_buffer_overrun_main output argument | main.cpp:9:29:9:32 | *argv | provenance |  |
@@ -26,13 +24,10 @@ edges
 | main.cpp:9:29:9:32 | **argv | main.cpp:9:29:9:32 | tests_restrict_main output argument | provenance |  |
 | main.cpp:9:29:9:32 | **argv | tests_restrict.c:15:41:15:44 | **argv | provenance |  |
 | main.cpp:9:29:9:32 | *argv | main.cpp:9:29:9:32 | tests_restrict_main output argument | provenance |  |
-| main.cpp:9:29:9:32 | *argv | main.cpp:9:29:9:32 | tests_restrict_main output argument | provenance |  |
-| main.cpp:9:29:9:32 | *argv | tests_restrict.c:15:41:15:44 | **argv | provenance |  |
 | main.cpp:9:29:9:32 | *argv | tests_restrict.c:15:41:15:44 | *argv | provenance |  |
 | main.cpp:9:29:9:32 | tests_restrict_main output argument | main.cpp:10:20:10:23 | **argv | provenance |  |
 | main.cpp:9:29:9:32 | tests_restrict_main output argument | main.cpp:10:20:10:23 | *argv | provenance |  |
 | main.cpp:10:20:10:23 | **argv | tests.cpp:689:32:689:35 | **argv | provenance |  |
-| main.cpp:10:20:10:23 | *argv | tests.cpp:689:32:689:35 | **argv | provenance |  |
 | main.cpp:10:20:10:23 | *argv | tests.cpp:689:32:689:35 | *argv | provenance |  |
 | overflowdestination.cpp:23:45:23:48 | **argv | overflowdestination.cpp:23:45:23:48 | **argv | provenance |  |
 | overflowdestination.cpp:23:45:23:48 | **argv | overflowdestination.cpp:23:45:23:48 | *argv | provenance |  |
@@ -98,11 +93,8 @@ subpaths
 | main.cpp:7:33:7:36 | **argv | overflowdestination.cpp:23:45:23:48 | **argv | overflowdestination.cpp:23:45:23:48 | *argv | main.cpp:7:33:7:36 | overflowdesination_main output argument |
 | main.cpp:8:34:8:37 | **argv | test_buffer_overrun.cpp:32:46:32:49 | **argv | test_buffer_overrun.cpp:32:46:32:49 | **argv | main.cpp:8:34:8:37 | test_buffer_overrun_main output argument |
 | main.cpp:8:34:8:37 | **argv | test_buffer_overrun.cpp:32:46:32:49 | **argv | test_buffer_overrun.cpp:32:46:32:49 | *argv | main.cpp:8:34:8:37 | test_buffer_overrun_main output argument |
-| main.cpp:8:34:8:37 | *argv | test_buffer_overrun.cpp:32:46:32:49 | **argv | test_buffer_overrun.cpp:32:46:32:49 | **argv | main.cpp:8:34:8:37 | test_buffer_overrun_main output argument |
-| main.cpp:8:34:8:37 | *argv | test_buffer_overrun.cpp:32:46:32:49 | **argv | test_buffer_overrun.cpp:32:46:32:49 | *argv | main.cpp:8:34:8:37 | test_buffer_overrun_main output argument |
 | main.cpp:8:34:8:37 | *argv | test_buffer_overrun.cpp:32:46:32:49 | *argv | test_buffer_overrun.cpp:32:46:32:49 | *argv | main.cpp:8:34:8:37 | test_buffer_overrun_main output argument |
 | main.cpp:9:29:9:32 | **argv | tests_restrict.c:15:41:15:44 | **argv | tests_restrict.c:15:41:15:44 | **argv | main.cpp:9:29:9:32 | tests_restrict_main output argument |
-| main.cpp:9:29:9:32 | *argv | tests_restrict.c:15:41:15:44 | **argv | tests_restrict.c:15:41:15:44 | **argv | main.cpp:9:29:9:32 | tests_restrict_main output argument |
 | main.cpp:9:29:9:32 | *argv | tests_restrict.c:15:41:15:44 | *argv | tests_restrict.c:15:41:15:44 | *argv | main.cpp:9:29:9:32 | tests_restrict_main output argument |
 #select
 | tests.cpp:615:2:615:7 | call to strcpy | main.cpp:6:27:6:30 | **argv | tests.cpp:615:17:615:22 | *source | This 'call to strcpy' with input from $@ may overflow the destination. | main.cpp:6:27:6:30 | **argv | a command-line argument |