From 5eeb5c6e679b162f9cc8662cc9367bd23f7922f0 Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jbj@github.com>
Date: Thu, 23 Jan 2020 15:28:42 +0100
Subject: [PATCH 1/2] C++: Use asExpr, not getConvertedResultExpression

We designed the IR's `DataFlow::Node.asExpr` very carefully so that it's
suitable for taint tracking, but then we didn't use it in
`DefaultTaintTracking.qll`. This meant that the sources in
`ArithmeticWithExtremeValues.ql` didn't get associated with any
`Instruction` and thus didn't propagate anywhere.

With this commit, the mapping of `Expr`-based sources to IR data-flow
nodes uses `asExpr`.
---
 .../cpp/ir/dataflow/DefaultTaintTracking.qll  | 42 +++++++++----------
 1 file changed, 19 insertions(+), 23 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll
index 0af802a255b9..b605e1650b5c 100644
--- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll
+++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll
@@ -19,33 +19,29 @@ private predicate predictableInstruction(Instruction instr) {
   predictableInstruction(instr.(UnaryInstruction).getUnary())
 }
 
-private predicate userInputInstruction(Instruction instr) {
-  exists(CallInstruction ci, WriteSideEffectInstruction wsei |
-    userInputArgument(ci.getConvertedResultExpression(), wsei.getIndex()) and
-    instr = wsei and
-    wsei.getPrimaryInstruction() = ci
-  )
-  or
-  userInputReturned(instr.getConvertedResultExpression())
-  or
-  isUserInput(instr.getConvertedResultExpression(), _)
-  or
-  instr.getConvertedResultExpression() instanceof EnvironmentRead
-  or
-  instr
-      .(LoadInstruction)
-      .getSourceAddress()
-      .(VariableAddressInstruction)
-      .getASTVariable()
-      .hasName("argv") and
-  instr.getEnclosingFunction().hasGlobalName("main")
-}
-
 private class DefaultTaintTrackingCfg extends DataFlow::Configuration {
   DefaultTaintTrackingCfg() { this = "DefaultTaintTrackingCfg" }
 
   override predicate isSource(DataFlow::Node source) {
-    userInputInstruction(source.asInstruction())
+    exists(CallInstruction ci, WriteSideEffectInstruction wsei |
+      userInputArgument(ci.getConvertedResultExpression(), wsei.getIndex()) and
+      source.asInstruction() = wsei and
+      wsei.getPrimaryInstruction() = ci
+    )
+    or
+    userInputReturned(source.asExpr())
+    or
+    isUserInput(source.asExpr(), _)
+    or
+    source.asExpr() instanceof EnvironmentRead
+    or
+    source.asInstruction()
+        .(LoadInstruction)
+        .getSourceAddress()
+        .(VariableAddressInstruction)
+        .getASTVariable()
+        .hasName("argv") and
+    source.asInstruction().getEnclosingFunction().hasGlobalName("main")
   }
 
   override predicate isSink(DataFlow::Node sink) { any() }

From 6606b2e18ae6f7db7bb2b8bd3fcbf9d0e27a3a54 Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jbj@github.com>
Date: Fri, 24 Jan 2020 10:48:03 +0100
Subject: [PATCH 2/2] C++: autoformat fixup

---
 .../src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll   | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll
index b605e1650b5c..6bfef8a61030 100644
--- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll
+++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll
@@ -35,7 +35,8 @@ private class DefaultTaintTrackingCfg extends DataFlow::Configuration {
     or
     source.asExpr() instanceof EnvironmentRead
     or
-    source.asInstruction()
+    source
+        .asInstruction()
         .(LoadInstruction)
         .getSourceAddress()
         .(VariableAddressInstruction)