C++: Map operand nodes that are only used once onto the related instruction node

Also revert the changes to default taint tracking and `TaintedAllocationSize.ql`
as these are no longer needed with the above mapping.

Author: jketema

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

@@ -18,7 +18,12 @@ private module Cached {
       // node for it.
       (not i.getResultType() instanceof VoidType or i.isGLValue())
     } or
-    TOperandNode0(Operand op) { not Ssa::ignoreOperand(op) }
+    TMultipleUseOperandNode0(Operand op) {
+      not Ssa::ignoreOperand(op) and not exists(Ssa::getIRRepresentationOfOperand(op))
+    } or
+    TSingleUseOperandNode0(Operand op) {
+      not Ssa::ignoreOperand(op) and exists(Ssa::getIRRepresentationOfOperand(op))
+    }
 }
 
 private import Cached
@@ -44,7 +49,10 @@ class Node0Impl extends TIRDataFlowNode0 {
   Instruction asInstruction() { result = this.(InstructionNode0).getInstruction() }
 
   /** Gets the operands corresponding to this node, if any. */
-  Operand asOperand() { result = this.(OperandNode0).getOperand() }
+  Operand asOperand() {
+    result = this.(MultipleUseOperandNode0).getOperand() or
+    result = this.(SingleUseOperandNode0).getOperand()
+  }
 
   /** INTERNAL: Do not use. */
   Location getLocationImpl() {
@@ -92,11 +100,9 @@ class InstructionNode0 extends Node0Impl, TInstructionNode0 {
 /**
  * An operand, viewed as a node in a data flow graph.
  */
-class OperandNode0 extends Node0Impl, TOperandNode0 {
+abstract class OperandNode0 extends Node0Impl {
   Operand op;
 
-  OperandNode0() { this = TOperandNode0(op) }
-
   /** Gets the operand corresponding to this node. */
   Operand getOperand() { result = op }
 
@@ -111,6 +117,20 @@ class OperandNode0 extends Node0Impl, TOperandNode0 {
   override string toStringImpl() { result = this.getOperand().toString() }
 }
 
+/**
+ * An operand that is used multiple times, viewed as a node in a data flow graph.
+ */
+class MultipleUseOperandNode0 extends OperandNode0, TMultipleUseOperandNode0 {
+  MultipleUseOperandNode0() { this = TMultipleUseOperandNode0(op) }
+}
+
+/**
+ * An operand that is used only once, viewed as a node in a data flow graph.
+ */
+class SingleUseOperandNode0 extends OperandNode0, TSingleUseOperandNode0 {
+  SingleUseOperandNode0() { this = TSingleUseOperandNode0(op) }
+}
+
 /**
  * INTERNAL: Do not use.
  *

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

@@ -356,10 +356,16 @@ private class Node0 extends Node, TNode0 {
  * An instruction, viewed as a node in a data flow graph.
  */
 class InstructionNode extends Node0 {
-  override InstructionNode0 node;
   Instruction instr;
 
-  InstructionNode() { instr = node.getInstruction() }
+  InstructionNode() {
+    node.(InstructionNode0).getInstruction() = instr
+    or
+    exists(Operand op |
+      instr = Ssa::getIRRepresentationOfOperand(op) and
+      node.(SingleUseOperandNode0).getOperand() = op
+    )
+  }
 
   /** Gets the instruction corresponding to this node. */
   Instruction getInstruction() { result = instr }
@@ -377,7 +383,7 @@ class OperandNode extends Node, Node0 {
   OperandNode() { op = node.getOperand() }
 
   /** Gets the operand corresponding to this node. */
-  Operand getOperand() { result = node.getOperand() }
+  Operand getOperand() { result = op }
 
   override string toStringImpl() { result = op.getDef().getAst().toString() }
 }

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

@@ -168,18 +168,16 @@ private predicate hasUpperBoundsCheck(Variable var) {
 private predicate nodeIsBarrierEqualityCandidate(
   DataFlow::Node node, Operand access, Variable checkedVar
 ) {
-  exists(Instruction instr | instr = node.asOperand().getDef() |
-    readsVariable(instr, checkedVar) and
-    any(IRGuardCondition guard).ensuresEq(access, _, _, instr.getBlock(), true)
-  )
+  readsVariable(node.asInstruction(), checkedVar) and
+  any(IRGuardCondition guard).ensuresEq(access, _, _, node.asInstruction().getBlock(), true)
 }
 
 cached
 private module Cached {
   cached
   predicate nodeIsBarrier(DataFlow::Node node) {
-    exists(Variable checkedVar, Instruction instr | instr = node.asOperand().getDef() |
-      readsVariable(instr, checkedVar) and
+    exists(Variable checkedVar |
+      readsVariable(node.asInstruction(), checkedVar) and
       hasUpperBoundsCheck(checkedVar)
     )
     or

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

@@ -607,6 +607,18 @@ private module Cached {
     )
   }
 
+  /**
+   * Holds if the underlying IR has a suitable instruction to represent a value
+   * that would otherwise need to be represented by a dedicated `OperandNode` value.
+   *
+   * Such operands do not create new `OperandNode` values, but are
+   * instead associated with the instruction returned by this predicate.
+   */
+  cached
+  Instruction getIRRepresentationOfOperand(Operand operand) {
+    operand = unique( | | result.getAUse())
+  }
+
   /**
    * Holds if the underlying IR has a suitable operand to represent a value
    * that would otherwise need to be represented by a dedicated `RawIndirectOperand` value.
@@ -627,7 +639,7 @@ private module Cached {
    * Holds if the underlying IR has a suitable instruction to represent a value
    * that would otherwise need to be represented by a dedicated `RawIndirectInstruction` value.
    *
-   * Such instruction do not create new `RawIndirectOperand` values, but are
+   * Such instructions do not create new `RawIndirectOperand` values, but are
    * instead associated with the instruction returned by this predicate.
    */
   cached

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

@@ -46,10 +46,8 @@ predicate hasUpperBoundsCheck(Variable var) {
 }
 
 predicate nodeIsBarrierEqualityCandidate(DataFlow::Node node, Operand access, Variable checkedVar) {
-  exists(Instruction instr | instr = node.asOperand().getDef() |
-    readsVariable(instr, checkedVar) and
-    any(IRGuardCondition guard).ensuresEq(access, _, _, instr.getBlock(), true)
-  )
+  readsVariable(node.asInstruction(), checkedVar) and
+  any(IRGuardCondition guard).ensuresEq(access, _, _, node.asInstruction().getBlock(), true)
 }
 
 predicate isFlowSource(FlowSource source, string sourceType) { sourceType = source.getSourceType() }
@@ -81,8 +79,8 @@ class TaintedAllocationSizeConfiguration extends TaintTracking::Configuration {
       e = any(PointerDiffExpr diff).getAnOperand()
     )
     or
-    exists(Variable checkedVar, Instruction instr | instr = node.asOperand().getDef() |
-      readsVariable(instr, checkedVar) and
+    exists(Variable checkedVar |
+      readsVariable(node.asInstruction(), checkedVar) and
       hasUpperBoundsCheck(checkedVar)
     )
     or

cpp/ql/test/query-tests/Security/CWE/CWE-078/semmle/ExecTainted/ExecTainted.expected

@@ -16,16 +16,20 @@ edges
 | test.cpp:93:11:93:14 | strncat output argument | test.cpp:94:45:94:48 | array to pointer conversion indirection |
 | test.cpp:93:17:93:24 | (const char *)... indirection | test.cpp:93:11:93:14 | strncat output argument |
 | test.cpp:106:20:106:25 | call to getenv | test.cpp:107:33:107:36 | (reference to) indirection |
+| test.cpp:106:20:106:25 | call to getenv | test.cpp:107:33:107:36 | (reference to) indirection |
 | test.cpp:106:20:106:38 | call to getenv indirection | test.cpp:107:33:107:36 | (reference to) indirection |
 | test.cpp:107:31:107:31 | call to operator+ | test.cpp:108:18:108:22 | call to c_str indirection |
 | test.cpp:107:33:107:36 | (reference to) indirection | test.cpp:107:31:107:31 | call to operator+ |
+| test.cpp:107:33:107:36 | (reference to) indirection | test.cpp:108:18:108:22 | call to c_str indirection |
+| test.cpp:113:20:113:25 | call to getenv | test.cpp:114:19:114:22 | (reference to) indirection |
 | test.cpp:113:20:113:25 | call to getenv | test.cpp:114:19:114:22 | (reference to) indirection |
 | test.cpp:113:20:113:38 | call to getenv indirection | test.cpp:114:19:114:22 | (reference to) indirection |
 | test.cpp:114:10:114:23 | (const basic_string<char, char_traits<char>, allocator<char>>)... | test.cpp:114:25:114:29 | call to c_str indirection |
 | test.cpp:114:17:114:17 | call to operator+ | test.cpp:114:25:114:29 | call to c_str indirection |
 | test.cpp:114:19:114:22 | (reference to) indirection | test.cpp:114:10:114:23 | (const basic_string<char, char_traits<char>, allocator<char>>)... |
 | test.cpp:114:19:114:22 | (reference to) indirection | test.cpp:114:17:114:17 | call to operator+ |
 | test.cpp:119:20:119:25 | call to getenv | test.cpp:120:19:120:22 | (reference to) indirection |
+| test.cpp:119:20:119:25 | call to getenv | test.cpp:120:19:120:22 | (reference to) indirection |
 | test.cpp:119:20:119:38 | call to getenv indirection | test.cpp:120:19:120:22 | (reference to) indirection |
 | test.cpp:120:17:120:17 | call to operator+ | test.cpp:120:10:120:30 | call to data indirection |
 | test.cpp:120:19:120:22 | (reference to) indirection | test.cpp:120:17:120:17 | call to operator+ |
@@ -123,18 +127,21 @@ nodes
 | test.cpp:93:17:93:24 | (const char *)... indirection | semmle.label | (const char *)... indirection |
 | test.cpp:94:45:94:48 | array to pointer conversion indirection | semmle.label | array to pointer conversion indirection |
 | test.cpp:106:20:106:25 | call to getenv | semmle.label | call to getenv |
+| test.cpp:106:20:106:25 | call to getenv | semmle.label | call to getenv |
 | test.cpp:106:20:106:38 | call to getenv indirection | semmle.label | call to getenv indirection |
 | test.cpp:107:31:107:31 | call to operator+ | semmle.label | call to operator+ |
 | test.cpp:107:33:107:36 | (reference to) indirection | semmle.label | (reference to) indirection |
 | test.cpp:108:18:108:22 | call to c_str indirection | semmle.label | call to c_str indirection |
 | test.cpp:113:20:113:25 | call to getenv | semmle.label | call to getenv |
+| test.cpp:113:20:113:25 | call to getenv | semmle.label | call to getenv |
 | test.cpp:113:20:113:38 | call to getenv indirection | semmle.label | call to getenv indirection |
 | test.cpp:114:10:114:23 | (const basic_string<char, char_traits<char>, allocator<char>>)... | semmle.label | (const basic_string<char, char_traits<char>, allocator<char>>)... |
 | test.cpp:114:17:114:17 | call to operator+ | semmle.label | call to operator+ |
 | test.cpp:114:19:114:22 | (reference to) indirection | semmle.label | (reference to) indirection |
 | test.cpp:114:25:114:29 | call to c_str indirection | semmle.label | call to c_str indirection |
 | test.cpp:114:25:114:29 | call to c_str indirection | semmle.label | call to c_str indirection |
 | test.cpp:119:20:119:25 | call to getenv | semmle.label | call to getenv |
+| test.cpp:119:20:119:25 | call to getenv | semmle.label | call to getenv |
 | test.cpp:119:20:119:38 | call to getenv indirection | semmle.label | call to getenv indirection |
 | test.cpp:120:10:120:30 | call to data indirection | semmle.label | call to data indirection |
 | test.cpp:120:17:120:17 | call to operator+ | semmle.label | call to operator+ |
@@ -218,12 +225,19 @@ subpaths
 | test.cpp:85:32:85:38 | command | test.cpp:82:9:82:16 | fread output argument | test.cpp:85:32:85:38 | array to pointer conversion indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to execl. | test.cpp:82:9:82:16 | fread output argument | user input (string read by fread) | test.cpp:84:11:84:17 | strncat output argument | strncat output argument |
 | test.cpp:94:45:94:48 | path | test.cpp:91:9:91:16 | fread output argument | test.cpp:94:45:94:48 | array to pointer conversion indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to execl. | test.cpp:91:9:91:16 | fread output argument | user input (string read by fread) | test.cpp:93:11:93:14 | strncat output argument | strncat output argument |
 | test.cpp:108:18:108:22 | call to c_str | test.cpp:106:20:106:25 | call to getenv | test.cpp:108:18:108:22 | call to c_str indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:106:20:106:25 | call to getenv | user input (an environment variable) | test.cpp:107:31:107:31 | call to operator+ | call to operator+ |
+| test.cpp:108:18:108:22 | call to c_str | test.cpp:106:20:106:25 | call to getenv | test.cpp:108:18:108:22 | call to c_str indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:106:20:106:25 | call to getenv | user input (an environment variable) | test.cpp:107:31:107:31 | call to operator+ | call to operator+ |
+| test.cpp:108:18:108:22 | call to c_str | test.cpp:106:20:106:25 | call to getenv | test.cpp:108:18:108:22 | call to c_str indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:106:20:106:25 | call to getenv | user input (an environment variable) | test.cpp:107:31:107:31 | call to operator+ | call to operator+ |
+| test.cpp:108:18:108:22 | call to c_str | test.cpp:106:20:106:25 | call to getenv | test.cpp:108:18:108:22 | call to c_str indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:106:20:106:25 | call to getenv | user input (an environment variable) | test.cpp:107:31:107:31 | call to operator+ | call to operator+ |
+| test.cpp:108:18:108:22 | call to c_str | test.cpp:106:20:106:38 | call to getenv indirection | test.cpp:108:18:108:22 | call to c_str indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:106:20:106:38 | call to getenv indirection | user input (an environment variable) | test.cpp:107:31:107:31 | call to operator+ | call to operator+ |
 | test.cpp:108:18:108:22 | call to c_str | test.cpp:106:20:106:38 | call to getenv indirection | test.cpp:108:18:108:22 | call to c_str indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:106:20:106:38 | call to getenv indirection | user input (an environment variable) | test.cpp:107:31:107:31 | call to operator+ | call to operator+ |
 | test.cpp:114:25:114:29 | call to c_str | test.cpp:113:20:113:25 | call to getenv | test.cpp:114:25:114:29 | call to c_str indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:113:20:113:25 | call to getenv | user input (an environment variable) | test.cpp:114:10:114:23 | (const basic_string<char, char_traits<char>, allocator<char>>)... | (const basic_string<char, char_traits<char>, allocator<char>>)... |
+| test.cpp:114:25:114:29 | call to c_str | test.cpp:113:20:113:25 | call to getenv | test.cpp:114:25:114:29 | call to c_str indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:113:20:113:25 | call to getenv | user input (an environment variable) | test.cpp:114:10:114:23 | (const basic_string<char, char_traits<char>, allocator<char>>)... | (const basic_string<char, char_traits<char>, allocator<char>>)... |
+| test.cpp:114:25:114:29 | call to c_str | test.cpp:113:20:113:25 | call to getenv | test.cpp:114:25:114:29 | call to c_str indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:113:20:113:25 | call to getenv | user input (an environment variable) | test.cpp:114:17:114:17 | call to operator+ | call to operator+ |
 | test.cpp:114:25:114:29 | call to c_str | test.cpp:113:20:113:25 | call to getenv | test.cpp:114:25:114:29 | call to c_str indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:113:20:113:25 | call to getenv | user input (an environment variable) | test.cpp:114:17:114:17 | call to operator+ | call to operator+ |
 | test.cpp:114:25:114:29 | call to c_str | test.cpp:113:20:113:38 | call to getenv indirection | test.cpp:114:25:114:29 | call to c_str indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:113:20:113:38 | call to getenv indirection | user input (an environment variable) | test.cpp:114:10:114:23 | (const basic_string<char, char_traits<char>, allocator<char>>)... | (const basic_string<char, char_traits<char>, allocator<char>>)... |
 | test.cpp:114:25:114:29 | call to c_str | test.cpp:113:20:113:38 | call to getenv indirection | test.cpp:114:25:114:29 | call to c_str indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:113:20:113:38 | call to getenv indirection | user input (an environment variable) | test.cpp:114:17:114:17 | call to operator+ | call to operator+ |
 | test.cpp:120:25:120:28 | call to data | test.cpp:119:20:119:25 | call to getenv | test.cpp:120:10:120:30 | call to data indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:119:20:119:25 | call to getenv | user input (an environment variable) | test.cpp:120:17:120:17 | call to operator+ | call to operator+ |
+| test.cpp:120:25:120:28 | call to data | test.cpp:119:20:119:25 | call to getenv | test.cpp:120:10:120:30 | call to data indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:119:20:119:25 | call to getenv | user input (an environment variable) | test.cpp:120:17:120:17 | call to operator+ | call to operator+ |
 | test.cpp:120:25:120:28 | call to data | test.cpp:119:20:119:38 | call to getenv indirection | test.cpp:120:10:120:30 | call to data indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:119:20:119:38 | call to getenv indirection | user input (an environment variable) | test.cpp:120:17:120:17 | call to operator+ | call to operator+ |
 | test.cpp:143:10:143:16 | command | test.cpp:140:9:140:11 | fread output argument | test.cpp:143:10:143:16 | (const char *)... indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:140:9:140:11 | fread output argument | user input (string read by fread) | test.cpp:142:11:142:17 | sprintf output argument | sprintf output argument |
 | test.cpp:183:32:183:38 | command | test.cpp:174:9:174:16 | fread output argument | test.cpp:183:32:183:38 | array to pointer conversion indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to execl. | test.cpp:174:9:174:16 | fread output argument | user input (string read by fread) | test.cpp:177:13:177:17 | strncat output argument | strncat output argument |