Fix Allocation Size Overflow for use-use flow

We have an operator expression like `x * 5`. We want to follow where the value of the operator expression goes. We used to follow local flow from an operand, but now there is flow from that operand to the next use of the variable. The fix is to explicitly start local flow from the operator expression. There are also some expected edge changes due to use-use flow.
github · owen-mc · Nov 10, 2023 · Nov 28, 2023 · Nov 28, 2023 · Nov 28, 2023
commit ad0b836fe579e2b654ea36ac7fee0ceed6ec39d8
@@ -32,7 +32,10 @@ module AllocationSizeOverflow {
  /**
   * A data-flow node that is an operand to an operation that may overflow.
   */
-  abstract class OverflowProneOperand extends DataFlow::Node { }
+  abstract class OverflowProneOperand extends DataFlow::Node {
+    /** Gets the operation that may overflow that `this` is an operand of. */
+    abstract DataFlow::Node getOverflowProneOperation();
+  }

  /**
   * A data-flow node that represents the size argument of an allocation, such as the `n` in
@@ -91,8 +94,7 @@ module AllocationSizeOverflow {
    AllocationSize allocsz;

    DefaultSink() {
-      this instanceof OverflowProneOperand and
-      localStep*(this, allocsz) and
+      localStep*(this.(OverflowProneOperand).getOverflowProneOperation(), allocsz) and
      not allocsz instanceof AllocationSizeCheckBarrier
    }

@@ -134,15 +136,18 @@ module AllocationSizeOverflow {

  /** An operand of an arithmetic expression that could cause overflow. */
  private class DefaultOverflowProneOperand extends OverflowProneOperand {
+    OperatorExpr parent;
+
    DefaultOverflowProneOperand() {
-      exists(OperatorExpr parent | isOverflowProne(parent) |
-        this.asExpr() = parent.getAnOperand() and
-        // only consider outermost operands to avoid double reporting
-        not exists(OperatorExpr grandparent | parent = grandparent.getAnOperand().stripParens() |
-          isOverflowProne(grandparent)
-        )
+      isOverflowProne(parent) and
+      this.asExpr() = parent.getAnOperand() and
+      // only consider outermost operands to avoid double reporting
+      not exists(OperatorExpr grandparent | parent = grandparent.getAnOperand().stripParens() |
+        isOverflowProne(grandparent)
      )
    }
+
+    override DataFlow::Node getOverflowProneOperation() { result.asExpr() = parent }
  }

  /**

@@ -17,10 +17,13 @@ edges
 | tst2.go:14:2:14:29 | ... := ...[0] | tst2.go:15:26:15:29 | data | provenance |  |
 | tst2.go:15:26:15:29 | data | tst2.go:15:22:15:30 | call to len | provenance | Config |
 | tst3.go:6:2:6:31 | ... := ...[0] | tst3.go:7:26:7:33 | jsonData | provenance |  |
-| tst3.go:6:2:6:31 | ... := ...[0] | tst3.go:24:20:24:27 | jsonData | provenance |  |
-| tst3.go:6:2:6:31 | ... := ...[0] | tst3.go:32:20:32:27 | jsonData | provenance |  |
 | tst3.go:7:26:7:33 | jsonData | tst3.go:7:22:7:34 | call to len | provenance | Config |
+| tst3.go:7:26:7:33 | jsonData | tst3.go:9:32:9:39 | jsonData | provenance |  |
+| tst3.go:9:32:9:39 | jsonData | tst3.go:11:9:11:16 | jsonData | provenance |  |
+| tst3.go:11:9:11:16 | jsonData | tst3.go:16:20:16:27 | jsonData | provenance |  |
+| tst3.go:16:20:16:27 | jsonData | tst3.go:24:20:24:27 | jsonData | provenance |  |
 | tst3.go:24:20:24:27 | jsonData | tst3.go:24:16:24:28 | call to len | provenance | Config |
+| tst3.go:24:20:24:27 | jsonData | tst3.go:32:20:32:27 | jsonData | provenance |  |
 | tst3.go:32:20:32:27 | jsonData | tst3.go:32:16:32:28 | call to len | provenance | Config |
 | tst.go:14:2:14:30 | ... = ...[0] | tst.go:15:26:15:33 | jsonData | provenance |  |
 | tst.go:15:26:15:33 | jsonData | tst.go:15:22:15:34 | call to len | provenance | Config |
@@ -45,6 +48,9 @@ nodes
 | tst3.go:6:2:6:31 | ... := ...[0] | semmle.label | ... := ...[0] |
 | tst3.go:7:22:7:34 | call to len | semmle.label | call to len |
 | tst3.go:7:26:7:33 | jsonData | semmle.label | jsonData |
+| tst3.go:9:32:9:39 | jsonData | semmle.label | jsonData |
+| tst3.go:11:9:11:16 | jsonData | semmle.label | jsonData |
+| tst3.go:16:20:16:27 | jsonData | semmle.label | jsonData |
 | tst3.go:24:16:24:28 | call to len | semmle.label | call to len |
 | tst3.go:24:20:24:27 | jsonData | semmle.label | jsonData |
 | tst3.go:32:16:32:28 | call to len | semmle.label | call to len |