From 185d0910c3f64e53491fdb73d4a5047ab9c19711 Mon Sep 17 00:00:00 2001
From: Max Schaefer <max@semmle.com>
Date: Thu, 5 Mar 2020 11:23:06 +0000
Subject: [PATCH 1/8] Sharpen `stringConcatStep` to exclude addition.

---
 ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll      | 3 ++-
 .../semmle/go/dataflow/FlowSteps/LocalTaintStep.expected      | 4 ----
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll b/ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll
index 73ff6b256d9..23ec8757d10 100644
--- a/ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll
+++ b/ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll
@@ -83,7 +83,8 @@ predicate tupleStep(DataFlow::Node pred, DataFlow::Node succ) {
 
 /** Holds if taint flows from `pred` to `succ` via string concatenation. */
 predicate stringConcatStep(DataFlow::Node pred, DataFlow::Node succ) {
-  exists(DataFlow::BinaryOperationNode conc | conc.getOperator() = "+" |
+  exists(DataFlow::BinaryOperationNode conc |
+    conc.getOperator() = "+" and conc.getType() instanceof StringType |
     succ = conc and conc.getAnOperand() = pred
   )
 }
diff --git a/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalTaintStep.expected b/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalTaintStep.expected
index 5d8e6e5610e..3cb26397561 100644
--- a/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalTaintStep.expected
+++ b/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalTaintStep.expected
@@ -1,7 +1,3 @@
-| main.go:10:22:10:22 | x | main.go:10:22:10:27 | ...+... |
-| main.go:10:24:10:27 | call to fn | main.go:10:22:10:27 | ...+... |
-| main.go:17:3:17:5 | acc | main.go:17:3:17:7 | rhs of increment statement |
-| main.go:17:3:17:7 | 1 | main.go:17:3:17:7 | rhs of increment statement |
 | main.go:26:11:26:17 | type assertion | main.go:26:2:26:17 | ... := ...[0] |
 | main.go:26:11:26:17 | type assertion | main.go:26:2:26:17 | ... := ...[1] |
 | strings.go:9:24:9:24 | s | strings.go:9:8:9:38 | call to Replace |

From af2c7aae5d1cd161a352f1b6d0779b2b06651620 Mon Sep 17 00:00:00 2001
From: Max Schaefer <max@semmle.com>
Date: Thu, 5 Mar 2020 11:44:29 +0000
Subject: [PATCH 2/8] Don't rely on flow through function models in definition
 of `PostUpdateNode`.

---
 .../go/dataflow/internal/DataFlowUtil.qll     | 22 +++++++++++++------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll
index ba8edbe5561..0894c86855c 100644
--- a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll
+++ b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll
@@ -402,7 +402,7 @@ class PostUpdateNode extends Node {
       preupd = this.(SsaNode).getAUse()
       or
       preupd = this and
-      not exists(getAPredecessor())
+      not basicLocalFlowStep(_, this)
     )
   }
 
@@ -766,13 +766,10 @@ Node extractTupleElement(Node t, int i) {
 predicate localFlowStep(Node nodeFrom, Node nodeTo) { simpleLocalFlowStep(nodeFrom, nodeTo) }
 
 /**
- * INTERNAL: do not use.
- *
- * This is the local flow predicate that's used as a building block in global
- * data flow. It may have less flow than the `localFlowStep` predicate.
+ * Holds if data flows from `nodeFrom` to `nodeTo` in exactly one local
+ * (intra-procedural) step, not taking function models into account.
  */
-cached
-predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
+private predicate basicLocalFlowStep(Node nodeFrom, Node nodeTo) {
   // Instruction -> Instruction
   exists(Expr pred, Expr succ |
     succ.(LogicalBinaryExpr).getAnOperand() = pred or
@@ -808,6 +805,17 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
   or
   // GlobalFunctionNode -> use
   nodeTo = MkGlobalFunctionNode(nodeFrom.asExpr().(FunctionName).getTarget())
+}
+
+/**
+ * INTERNAL: do not use.
+ *
+ * This is the local flow predicate that's used as a building block in global
+ * data flow. It may have less flow than the `localFlowStep` predicate.
+ */
+cached
+predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
+  basicLocalFlowStep(nodeFrom, nodeTo)
   or
   // step through function model
   exists(FunctionModel m, CallNode c, FunctionInput inp, FunctionOutput outp |

From b99c63d180a95fd9d5a54434e69d6a42efaf2350 Mon Sep 17 00:00:00 2001
From: Max Schaefer <max@semmle.com>
Date: Thu, 5 Mar 2020 11:46:17 +0000
Subject: [PATCH 3/8] Factor out an auxiliary predicate.

---
 .../go/dataflow/internal/DataFlowUtil.qll      | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll
index 0894c86855c..adcadf3bf60 100644
--- a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll
+++ b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll
@@ -412,6 +412,17 @@ class PostUpdateNode extends Node {
   Node getPreUpdateNode() { result = preupd }
 }
 
+/**
+ * Gets the `i`th argument of call `c`, where the receiver of a method call
+ * counts as argument -1.
+ */
+private Node getArgument(CallNode c, int i) {
+  result = c.getArgument(i)
+  or
+  result = c.(MethodCallNode).getReceiver() and
+  i = -1
+}
+
 /**
  * A data-flow node that occurs as an argument in a call, including receiver arguments.
  */
@@ -419,12 +430,7 @@ class ArgumentNode extends Node {
   CallNode c;
   int i;
 
-  ArgumentNode() {
-    this = c.getArgument(i)
-    or
-    this = c.(MethodCallNode).getReceiver() and
-    i = -1
-  }
+  ArgumentNode() { this = getArgument(c, i) }
 
   /**
    * Holds if this argument occurs at the given position in the given call.

From 3f8d2117d81ae54247851ffce789b2317d05eb20 Mon Sep 17 00:00:00 2001
From: Max Schaefer <max@semmle.com>
Date: Thu, 5 Mar 2020 11:46:59 +0000
Subject: [PATCH 4/8] Introduce post-update nodes for arguments with a mutable
 type.

---
 .../go/dataflow/internal/DataFlowUtil.qll       | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll
index adcadf3bf60..65ea3a252c7 100644
--- a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll
+++ b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll
@@ -397,6 +397,9 @@ class PostUpdateNode extends Node {
         or
         preupd = base.(PointerDereferenceNode).getOperand()
       )
+      or
+      preupd instanceof ArgumentNode and
+      mutableType(preupd.getType())
     ) and
     (
       preupd = this.(SsaNode).getAUse()
@@ -455,6 +458,20 @@ class ArgumentNode extends Node {
   CallNode getCall() { result = c }
 }
 
+/**
+ * Holds if `tp` is a type that may (directly or indirectly) reference a memory location.
+ *
+ * If a value with a mutable type is passed to a function, the function could potentially
+ * mutate it or something it points to.
+ */
+predicate mutableType(Type tp) {
+  tp instanceof ArrayType or
+  tp instanceof SliceType or
+  tp instanceof MapType or
+  tp instanceof PointerType or
+  tp instanceof InterfaceType
+}
+
 /**
  * A node whose value is returned as a result from a function.
  *

From 4f061005cbd85d8c99a134eb506075d85733aef0 Mon Sep 17 00:00:00 2001
From: Max Schaefer <max@semmle.com>
Date: Thu, 5 Mar 2020 11:55:46 +0000
Subject: [PATCH 5/8] Add a taint-tracking model for `copy`.

---
 .../go/dataflow/FunctionInputsAndOutputs.qll  | 57 ++++++++++++++++++-
 ql/src/semmle/go/frameworks/Stdlib.qll        | 12 ++++
 .../dataflow/FlowSteps/LocalFlowStep.expected | 12 ++++
 .../FlowSteps/LocalTaintStep.expected         |  1 +
 .../semmle/go/dataflow/FlowSteps/main.go      |  8 +++
 5 files changed, 89 insertions(+), 1 deletion(-)

diff --git a/ql/src/semmle/go/dataflow/FunctionInputsAndOutputs.qll b/ql/src/semmle/go/dataflow/FunctionInputsAndOutputs.qll
index eba333693b3..9282532a2f7 100644
--- a/ql/src/semmle/go/dataflow/FunctionInputsAndOutputs.qll
+++ b/ql/src/semmle/go/dataflow/FunctionInputsAndOutputs.qll
@@ -97,9 +97,18 @@ private newtype TFunctionOutput =
     // one among several results
     exists(SignatureType s | exists(s.getResultType(index)))
   }
+  or
+  TOutParameter(int index) {
+    // the receiver parameter
+    index = -1
+    or
+    // another parameter
+    exists(SignatureType s | exists(s.getParameterType(index)))
+  }
 
 /**
- * An abstract representation of an output of a function, which is one of its results.
+ * An abstract representation of an output of a function, which is one of its results
+ * or a parameter with mutable type.
  */
 class FunctionOutput extends TFunctionOutput {
   /** Holds if this represents the (single) result of a function. */
@@ -112,6 +121,16 @@ class FunctionOutput extends TFunctionOutput {
     none()
   }
 
+  /** Holds if this represents the receiver of a function. */
+  predicate isReceiver() {
+    none()
+  }
+
+  /** Holds if this represents the `i`th parameter of a function. */
+  predicate isParameter(int i) {
+    none()
+  }
+
   /** Gets the data-flow node corresponding to this output for the call `c`. */
   final DataFlow::Node getNode(DataFlow::CallNode c) { result = getExitNode(c) }
 
@@ -176,3 +195,39 @@ private class OutResult extends FunctionOutput, TOutResult {
     index >= 0 and result = "result " + index
   }
 }
+
+/** A result position of a function, viewed as an output. */
+private class OutParameter extends FunctionOutput, TOutParameter {
+  int index;
+
+  OutParameter() {
+    this = TOutParameter(index)
+  }
+
+  override predicate isReceiver() {
+    index = -1
+  }
+
+  override predicate isParameter(int i) {
+    i = index and i >= 0
+  }
+
+  override DataFlow::Node getEntryNode(FuncDef f) {
+    // there is no generic way of assigning to a parameter; operations that taint a parameter
+    // have to be handled on a case-by-case basis
+    none()
+  }
+
+  override DataFlow::Node getExitNode(DataFlow::CallNode c) {
+    exists(DataFlow::ArgumentNode arg |
+      arg.argumentOf(c.asExpr(), index) and
+      result.(DataFlow::PostUpdateNode).getPreUpdateNode() = arg
+    )
+  }
+
+  override string toString() {
+    index = -1 and result = "result"
+    or
+    index >= 0 and result = "result " + index
+  }
+}
\ No newline at end of file
diff --git a/ql/src/semmle/go/frameworks/Stdlib.qll b/ql/src/semmle/go/frameworks/Stdlib.qll
index eafbd375589..2db3187bda8 100644
--- a/ql/src/semmle/go/frameworks/Stdlib.qll
+++ b/ql/src/semmle/go/frameworks/Stdlib.qll
@@ -13,6 +13,18 @@ class StringMethod extends TaintTracking::FunctionModel, Method {
   }
 }
 
+/**
+ * A model of the built-in `copy` function, which propagates taint from its second argument
+ * to its first.
+ */
+private class CopyFunction extends TaintTracking::FunctionModel {
+  CopyFunction() { this = Builtin::copy() }
+
+  override predicate hasTaintFlow(DataFlow::FunctionInput inp, DataFlow::FunctionOutput outp) {
+    inp.isParameter(1) and outp.isParameter(0)
+  }
+}
+
 /** Provides models of commonly used functions in the `path/filepath` package. */
 module PathFilePath {
   /** A path-manipulating function in the `path/filepath` package. */
diff --git a/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalFlowStep.expected b/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalFlowStep.expected
index 6ffaeb1a103..5bfafd43807 100644
--- a/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalFlowStep.expected
+++ b/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalFlowStep.expected
@@ -37,6 +37,18 @@
 | main.go:34:2:34:6 | test1 | main.go:3:6:3:10 | function test1 |
 | main.go:34:8:34:12 | test2 | main.go:14:6:14:10 | function test2 |
 | main.go:34:19:34:23 | test2 | main.go:14:6:14:10 | function test2 |
+| main.go:38:2:38:2 | definition of s | main.go:39:15:39:15 | s |
+| main.go:38:2:38:2 | definition of s | main.go:40:15:40:15 | s |
+| main.go:38:2:38:2 | definition of s | main.go:42:7:42:7 | s |
+| main.go:38:7:38:20 | composite literal | main.go:38:2:38:2 | definition of s |
+| main.go:39:2:39:3 | definition of s1 | main.go:40:18:40:19 | s1 |
+| main.go:39:8:39:13 | append | file://:0:0:0:0 | function append |
+| main.go:39:8:39:25 | call to append | main.go:39:2:39:3 | definition of s1 |
+| main.go:40:8:40:13 | append | file://:0:0:0:0 | function append |
+| main.go:41:2:41:3 | definition of s4 | main.go:42:10:42:11 | s4 |
+| main.go:41:8:41:11 | make | file://:0:0:0:0 | function make |
+| main.go:41:8:41:21 | call to make | main.go:41:2:41:3 | definition of s4 |
+| main.go:42:2:42:5 | copy | file://:0:0:0:0 | function copy |
 | strings.go:8:12:8:12 | argument corresponding to s | strings.go:8:12:8:12 | definition of s |
 | strings.go:8:12:8:12 | definition of s | strings.go:9:24:9:24 | s |
 | strings.go:8:12:8:12 | definition of s | strings.go:10:27:10:27 | s |
diff --git a/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalTaintStep.expected b/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalTaintStep.expected
index 3cb26397561..41f7601cc38 100644
--- a/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalTaintStep.expected
+++ b/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalTaintStep.expected
@@ -1,5 +1,6 @@
 | main.go:26:11:26:17 | type assertion | main.go:26:2:26:17 | ... := ...[0] |
 | main.go:26:11:26:17 | type assertion | main.go:26:2:26:17 | ... := ...[1] |
+| main.go:42:10:42:11 | s4 | main.go:38:2:38:2 | definition of s |
 | strings.go:9:24:9:24 | s | strings.go:9:8:9:38 | call to Replace |
 | strings.go:9:32:9:34 | "_" | strings.go:9:8:9:38 | call to Replace |
 | strings.go:10:27:10:27 | s | strings.go:10:8:10:42 | call to ReplaceAll |
diff --git a/ql/test/library-tests/semmle/go/dataflow/FlowSteps/main.go b/ql/test/library-tests/semmle/go/dataflow/FlowSteps/main.go
index 9bcde307e2e..a06ef15684f 100644
--- a/ql/test/library-tests/semmle/go/dataflow/FlowSteps/main.go
+++ b/ql/test/library-tests/semmle/go/dataflow/FlowSteps/main.go
@@ -33,3 +33,11 @@ func test3(b bool, x interface{}) string {
 func main() {
 	test1(test2()(), test2())
 }
+
+func test9() {
+	s := []int{1, 2, 3}
+	s1 := append(s, 4, 5, 6)
+	s2 := append(s, s1...)
+	s4 := make([]int, 4)
+	copy(s, s4)
+}

From a7ecb50a34055d2deeda301ff387be7af5474a07 Mon Sep 17 00:00:00 2001
From: Max Schaefer <max@semmle.com>
Date: Fri, 6 Mar 2020 09:35:03 +0000
Subject: [PATCH 6/8] Add taint-tracking model for `append`.

---
 ql/src/semmle/go/frameworks/Stdlib.qll               | 12 ++++++++++++
 .../go/dataflow/FlowSteps/LocalTaintStep.expected    |  6 ++++++
 2 files changed, 18 insertions(+)

diff --git a/ql/src/semmle/go/frameworks/Stdlib.qll b/ql/src/semmle/go/frameworks/Stdlib.qll
index 2db3187bda8..63d66ad5778 100644
--- a/ql/src/semmle/go/frameworks/Stdlib.qll
+++ b/ql/src/semmle/go/frameworks/Stdlib.qll
@@ -13,6 +13,18 @@ class StringMethod extends TaintTracking::FunctionModel, Method {
   }
 }
 
+/**
+ * A model of the built-in `append` function, which propagates taint from its arguments to its
+ * result.
+ */
+private class AppendFunction extends TaintTracking::FunctionModel {
+  AppendFunction() { this = Builtin::append() }
+
+  override predicate hasTaintFlow(DataFlow::FunctionInput inp, DataFlow::FunctionOutput outp) {
+    inp.isParameter(_) and outp.isResult()
+  }
+}
+
 /**
  * A model of the built-in `copy` function, which propagates taint from its second argument
  * to its first.
diff --git a/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalTaintStep.expected b/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalTaintStep.expected
index 41f7601cc38..405933d23b1 100644
--- a/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalTaintStep.expected
+++ b/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalTaintStep.expected
@@ -1,5 +1,11 @@
 | main.go:26:11:26:17 | type assertion | main.go:26:2:26:17 | ... := ...[0] |
 | main.go:26:11:26:17 | type assertion | main.go:26:2:26:17 | ... := ...[1] |
+| main.go:39:15:39:15 | s | main.go:39:8:39:25 | call to append |
+| main.go:39:18:39:18 | 4 | main.go:39:8:39:25 | call to append |
+| main.go:39:21:39:21 | 5 | main.go:39:8:39:25 | call to append |
+| main.go:39:24:39:24 | 6 | main.go:39:8:39:25 | call to append |
+| main.go:40:15:40:15 | s | main.go:40:8:40:23 | call to append |
+| main.go:40:18:40:19 | s1 | main.go:40:8:40:23 | call to append |
 | main.go:42:10:42:11 | s4 | main.go:38:2:38:2 | definition of s |
 | strings.go:9:24:9:24 | s | strings.go:9:8:9:38 | call to Replace |
 | strings.go:9:32:9:34 | "_" | strings.go:9:8:9:38 | call to Replace |

From 9bcbfb2911f7540547bfd344fc394791e3239088 Mon Sep 17 00:00:00 2001
From: Max Schaefer <max@semmle.com>
Date: Fri, 6 Mar 2020 09:41:06 +0000
Subject: [PATCH 7/8] Fix flow step from global functions to their use.

How does anything work.
---
 .../go/dataflow/internal/DataFlowUtil.qll     |  2 +-
 .../dataflow/FlowSteps/LocalFlowStep.expected | 86 +++++++++----------
 2 files changed, 44 insertions(+), 44 deletions(-)

diff --git a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll
index 65ea3a252c7..310dd496518 100644
--- a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll
+++ b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll
@@ -827,7 +827,7 @@ private predicate basicLocalFlowStep(Node nodeFrom, Node nodeTo) {
   )
   or
   // GlobalFunctionNode -> use
-  nodeTo = MkGlobalFunctionNode(nodeFrom.asExpr().(FunctionName).getTarget())
+  nodeFrom = MkGlobalFunctionNode(nodeTo.asExpr().(FunctionName).getTarget())
 }
 
 /**
diff --git a/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalFlowStep.expected b/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalFlowStep.expected
index 5bfafd43807..80acf80639f 100644
--- a/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalFlowStep.expected
+++ b/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalFlowStep.expected
@@ -1,3 +1,44 @@
+| file://:0:0:0:0 | function Encode | url.go:51:14:51:21 | selection of Encode |
+| file://:0:0:0:0 | function EscapedPath | url.go:28:14:28:26 | selection of EscapedPath |
+| file://:0:0:0:0 | function Get | url.go:52:14:52:18 | selection of Get |
+| file://:0:0:0:0 | function Hostname | url.go:29:14:29:23 | selection of Hostname |
+| file://:0:0:0:0 | function MarshalBinary | url.go:30:11:30:25 | selection of MarshalBinary |
+| file://:0:0:0:0 | function Parse | url.go:23:10:23:18 | selection of Parse |
+| file://:0:0:0:0 | function Parse | url.go:32:9:32:15 | selection of Parse |
+| file://:0:0:0:0 | function ParseQuery | url.go:50:10:50:23 | selection of ParseQuery |
+| file://:0:0:0:0 | function ParseRequestURI | url.go:27:9:27:27 | selection of ParseRequestURI |
+| file://:0:0:0:0 | function Password | url.go:43:11:43:21 | selection of Password |
+| file://:0:0:0:0 | function PathEscape | url.go:12:31:12:44 | selection of PathEscape |
+| file://:0:0:0:0 | function PathUnescape | url.go:12:14:12:29 | selection of PathUnescape |
+| file://:0:0:0:0 | function Port | url.go:33:14:33:19 | selection of Port |
+| file://:0:0:0:0 | function Println | url.go:28:2:28:12 | selection of Println |
+| file://:0:0:0:0 | function Println | url.go:29:2:29:12 | selection of Println |
+| file://:0:0:0:0 | function Println | url.go:31:2:31:12 | selection of Println |
+| file://:0:0:0:0 | function Println | url.go:33:2:33:12 | selection of Println |
+| file://:0:0:0:0 | function Println | url.go:34:2:34:12 | selection of Println |
+| file://:0:0:0:0 | function Println | url.go:35:2:35:12 | selection of Println |
+| file://:0:0:0:0 | function Println | url.go:44:2:44:12 | selection of Println |
+| file://:0:0:0:0 | function Println | url.go:45:2:45:12 | selection of Println |
+| file://:0:0:0:0 | function Println | url.go:51:2:51:12 | selection of Println |
+| file://:0:0:0:0 | function Println | url.go:52:2:52:12 | selection of Println |
+| file://:0:0:0:0 | function Query | url.go:34:14:34:20 | selection of Query |
+| file://:0:0:0:0 | function QueryEscape | url.go:14:32:14:46 | selection of QueryEscape |
+| file://:0:0:0:0 | function QueryUnescape | url.go:14:14:14:30 | selection of QueryUnescape |
+| file://:0:0:0:0 | function Replace | strings.go:9:8:9:22 | selection of Replace |
+| file://:0:0:0:0 | function ReplaceAll | strings.go:10:8:10:25 | selection of ReplaceAll |
+| file://:0:0:0:0 | function RequestURI | url.go:35:14:35:25 | selection of RequestURI |
+| file://:0:0:0:0 | function ResolveReference | url.go:36:6:36:23 | selection of ResolveReference |
+| file://:0:0:0:0 | function Sprint | strings.go:11:9:11:18 | selection of Sprint |
+| file://:0:0:0:0 | function Sprintf | strings.go:11:30:11:40 | selection of Sprintf |
+| file://:0:0:0:0 | function Sprintln | strings.go:11:54:11:65 | selection of Sprintln |
+| file://:0:0:0:0 | function User | url.go:41:8:41:15 | selection of User |
+| file://:0:0:0:0 | function UserPassword | url.go:42:7:42:22 | selection of UserPassword |
+| file://:0:0:0:0 | function Username | url.go:45:14:45:24 | selection of Username |
+| file://:0:0:0:0 | function append | main.go:39:8:39:13 | append |
+| file://:0:0:0:0 | function append | main.go:40:8:40:13 | append |
+| file://:0:0:0:0 | function copy | main.go:42:2:42:5 | copy |
+| file://:0:0:0:0 | function make | main.go:41:8:41:11 | make |
+| main.go:3:6:3:10 | function test1 | main.go:34:2:34:6 | test1 |
 | main.go:3:12:3:12 | argument corresponding to x | main.go:3:12:3:12 | definition of x |
 | main.go:3:12:3:12 | definition of x | main.go:5:5:5:5 | x |
 | main.go:3:12:3:12 | definition of x | main.go:6:7:6:7 | x |
@@ -17,6 +58,8 @@
 | main.go:10:7:10:27 | ...&&... | main.go:10:2:10:2 | definition of z |
 | main.go:10:17:10:27 | ...>=... | main.go:10:7:10:27 | ...&&... |
 | main.go:11:14:11:14 | z | main.go:11:9:11:15 | type conversion |
+| main.go:14:6:14:10 | function test2 | main.go:34:8:34:12 | test2 |
+| main.go:14:6:14:10 | function test2 | main.go:34:19:34:23 | test2 |
 | main.go:15:2:15:4 | definition of acc | main.go:16:9:16:9 | capture variable acc |
 | main.go:15:9:15:9 | 0 | main.go:15:2:15:4 | definition of acc |
 | main.go:16:9:16:9 | capture variable acc | main.go:17:3:17:5 | acc |
@@ -34,35 +77,23 @@
 | main.go:26:2:26:17 | ... := ...[1] | main.go:26:5:26:6 | definition of ok |
 | main.go:26:5:26:6 | definition of ok | main.go:27:5:27:6 | ok |
 | main.go:26:11:26:11 | x | main.go:26:11:26:17 | type assertion |
-| main.go:34:2:34:6 | test1 | main.go:3:6:3:10 | function test1 |
-| main.go:34:8:34:12 | test2 | main.go:14:6:14:10 | function test2 |
-| main.go:34:19:34:23 | test2 | main.go:14:6:14:10 | function test2 |
 | main.go:38:2:38:2 | definition of s | main.go:39:15:39:15 | s |
 | main.go:38:2:38:2 | definition of s | main.go:40:15:40:15 | s |
 | main.go:38:2:38:2 | definition of s | main.go:42:7:42:7 | s |
 | main.go:38:7:38:20 | composite literal | main.go:38:2:38:2 | definition of s |
 | main.go:39:2:39:3 | definition of s1 | main.go:40:18:40:19 | s1 |
-| main.go:39:8:39:13 | append | file://:0:0:0:0 | function append |
 | main.go:39:8:39:25 | call to append | main.go:39:2:39:3 | definition of s1 |
-| main.go:40:8:40:13 | append | file://:0:0:0:0 | function append |
 | main.go:41:2:41:3 | definition of s4 | main.go:42:10:42:11 | s4 |
-| main.go:41:8:41:11 | make | file://:0:0:0:0 | function make |
 | main.go:41:8:41:21 | call to make | main.go:41:2:41:3 | definition of s4 |
-| main.go:42:2:42:5 | copy | file://:0:0:0:0 | function copy |
 | strings.go:8:12:8:12 | argument corresponding to s | strings.go:8:12:8:12 | definition of s |
 | strings.go:8:12:8:12 | definition of s | strings.go:9:24:9:24 | s |
 | strings.go:8:12:8:12 | definition of s | strings.go:10:27:10:27 | s |
 | strings.go:9:2:9:3 | definition of s2 | strings.go:11:20:11:21 | s2 |
 | strings.go:9:2:9:3 | definition of s2 | strings.go:11:48:11:49 | s2 |
-| strings.go:9:8:9:22 | selection of Replace | file://:0:0:0:0 | function Replace |
 | strings.go:9:8:9:38 | call to Replace | strings.go:9:2:9:3 | definition of s2 |
 | strings.go:10:2:10:3 | definition of s3 | strings.go:11:24:11:25 | s3 |
 | strings.go:10:2:10:3 | definition of s3 | strings.go:11:67:11:68 | s3 |
-| strings.go:10:8:10:25 | selection of ReplaceAll | file://:0:0:0:0 | function ReplaceAll |
 | strings.go:10:8:10:42 | call to ReplaceAll | strings.go:10:2:10:3 | definition of s3 |
-| strings.go:11:9:11:18 | selection of Sprint | file://:0:0:0:0 | function Sprint |
-| strings.go:11:30:11:40 | selection of Sprintf | file://:0:0:0:0 | function Sprintf |
-| strings.go:11:54:11:65 | selection of Sprintln | file://:0:0:0:0 | function Sprintln |
 | url.go:8:12:8:12 | argument corresponding to b | url.go:8:12:8:12 | definition of b |
 | url.go:8:12:8:12 | definition of b | url.go:11:5:11:5 | b |
 | url.go:8:20:8:20 | argument corresponding to s | url.go:8:20:8:20 | definition of s |
@@ -72,14 +103,10 @@
 | url.go:12:3:12:48 | ... = ...[0] | url.go:12:3:12:5 | definition of res |
 | url.go:12:3:12:48 | ... = ...[1] | url.go:12:8:12:10 | definition of err |
 | url.go:12:8:12:10 | definition of err | url.go:16:5:16:5 | err = phi(def@12:8, def@14:8) |
-| url.go:12:14:12:29 | selection of PathUnescape | file://:0:0:0:0 | function PathUnescape |
-| url.go:12:31:12:44 | selection of PathEscape | file://:0:0:0:0 | function PathEscape |
 | url.go:14:3:14:5 | definition of res | url.go:16:5:16:5 | res = phi(def@12:3, def@14:3) |
 | url.go:14:3:14:50 | ... = ...[0] | url.go:14:3:14:5 | definition of res |
 | url.go:14:3:14:50 | ... = ...[1] | url.go:14:8:14:10 | definition of err |
 | url.go:14:8:14:10 | definition of err | url.go:16:5:16:5 | err = phi(def@12:8, def@14:8) |
-| url.go:14:14:14:30 | selection of QueryUnescape | file://:0:0:0:0 | function QueryUnescape |
-| url.go:14:32:14:46 | selection of QueryEscape | file://:0:0:0:0 | function QueryEscape |
 | url.go:16:5:16:5 | err = phi(def@12:8, def@14:8) | url.go:16:5:16:7 | err |
 | url.go:16:5:16:5 | res = phi(def@12:3, def@14:3) | url.go:19:9:19:11 | res |
 | url.go:22:12:22:12 | argument corresponding to i | url.go:22:12:22:12 | definition of i |
@@ -89,57 +116,30 @@
 | url.go:22:19:22:19 | definition of s | url.go:27:29:27:29 | s |
 | url.go:23:2:23:2 | definition of u | url.go:25:10:25:10 | u |
 | url.go:23:2:23:21 | ... := ...[0] | url.go:23:2:23:2 | definition of u |
-| url.go:23:10:23:18 | selection of Parse | file://:0:0:0:0 | function Parse |
 | url.go:27:2:27:2 | definition of u | url.go:28:14:28:14 | u |
 | url.go:27:2:27:2 | definition of u | url.go:29:14:29:14 | u |
 | url.go:27:2:27:2 | definition of u | url.go:30:11:30:11 | u |
 | url.go:27:2:27:2 | definition of u | url.go:32:9:32:9 | u |
 | url.go:27:2:27:30 | ... = ...[0] | url.go:27:2:27:2 | definition of u |
-| url.go:27:9:27:27 | selection of ParseRequestURI | file://:0:0:0:0 | function ParseRequestURI |
-| url.go:28:2:28:12 | selection of Println | file://:0:0:0:0 | function Println |
-| url.go:28:14:28:26 | selection of EscapedPath | file://:0:0:0:0 | function EscapedPath |
-| url.go:29:2:29:12 | selection of Println | file://:0:0:0:0 | function Println |
-| url.go:29:14:29:23 | selection of Hostname | file://:0:0:0:0 | function Hostname |
 | url.go:30:2:30:3 | definition of bs | url.go:31:14:31:15 | bs |
 | url.go:30:2:30:27 | ... := ...[0] | url.go:30:2:30:3 | definition of bs |
-| url.go:30:11:30:25 | selection of MarshalBinary | file://:0:0:0:0 | function MarshalBinary |
-| url.go:31:2:31:12 | selection of Println | file://:0:0:0:0 | function Println |
 | url.go:32:2:32:2 | definition of u | url.go:33:14:33:14 | u |
 | url.go:32:2:32:2 | definition of u | url.go:34:14:34:14 | u |
 | url.go:32:2:32:2 | definition of u | url.go:35:14:35:14 | u |
 | url.go:32:2:32:2 | definition of u | url.go:36:6:36:6 | u |
 | url.go:32:2:32:2 | definition of u | url.go:36:25:36:25 | u |
 | url.go:32:2:32:23 | ... = ...[0] | url.go:32:2:32:2 | definition of u |
-| url.go:32:9:32:15 | selection of Parse | file://:0:0:0:0 | function Parse |
-| url.go:33:2:33:12 | selection of Println | file://:0:0:0:0 | function Println |
-| url.go:33:14:33:19 | selection of Port | file://:0:0:0:0 | function Port |
-| url.go:34:2:34:12 | selection of Println | file://:0:0:0:0 | function Println |
-| url.go:34:14:34:20 | selection of Query | file://:0:0:0:0 | function Query |
-| url.go:35:2:35:12 | selection of Println | file://:0:0:0:0 | function Println |
-| url.go:35:14:35:25 | selection of RequestURI | file://:0:0:0:0 | function RequestURI |
 | url.go:36:2:36:2 | definition of u | url.go:37:9:37:9 | u |
-| url.go:36:6:36:23 | selection of ResolveReference | file://:0:0:0:0 | function ResolveReference |
 | url.go:36:6:36:26 | call to ResolveReference | url.go:36:2:36:2 | definition of u |
-| url.go:41:8:41:15 | selection of User | file://:0:0:0:0 | function User |
 | url.go:42:2:42:3 | definition of ui | url.go:43:11:43:12 | ui |
 | url.go:42:2:42:3 | definition of ui | url.go:45:14:45:15 | ui |
 | url.go:42:2:42:3 | definition of ui | url.go:46:9:46:10 | ui |
-| url.go:42:7:42:22 | selection of UserPassword | file://:0:0:0:0 | function UserPassword |
 | url.go:42:7:42:38 | call to UserPassword | url.go:42:2:42:3 | definition of ui |
 | url.go:43:2:43:3 | definition of pw | url.go:44:14:44:15 | pw |
 | url.go:43:2:43:23 | ... := ...[0] | url.go:43:2:43:3 | definition of pw |
-| url.go:43:11:43:21 | selection of Password | file://:0:0:0:0 | function Password |
-| url.go:44:2:44:12 | selection of Println | file://:0:0:0:0 | function Println |
-| url.go:45:2:45:12 | selection of Println | file://:0:0:0:0 | function Println |
-| url.go:45:14:45:24 | selection of Username | file://:0:0:0:0 | function Username |
 | url.go:49:12:49:12 | argument corresponding to q | url.go:49:12:49:12 | definition of q |
 | url.go:49:12:49:12 | definition of q | url.go:50:25:50:25 | q |
 | url.go:50:2:50:2 | definition of v | url.go:51:14:51:14 | v |
 | url.go:50:2:50:2 | definition of v | url.go:52:14:52:14 | v |
 | url.go:50:2:50:2 | definition of v | url.go:53:9:53:9 | v |
 | url.go:50:2:50:26 | ... := ...[0] | url.go:50:2:50:2 | definition of v |
-| url.go:50:10:50:23 | selection of ParseQuery | file://:0:0:0:0 | function ParseQuery |
-| url.go:51:2:51:12 | selection of Println | file://:0:0:0:0 | function Println |
-| url.go:51:14:51:21 | selection of Encode | file://:0:0:0:0 | function Encode |
-| url.go:52:2:52:12 | selection of Println | file://:0:0:0:0 | function Println |
-| url.go:52:14:52:18 | selection of Get | file://:0:0:0:0 | function Get |

From aa8bc972d962df4ae7832bf547437f08eca09ed1 Mon Sep 17 00:00:00 2001
From: Max Schaefer <max@semmle.com>
Date: Fri, 6 Mar 2020 15:03:45 +0000
Subject: [PATCH 8/8] Address review comments.

---
 ql/src/semmle/go/dataflow/FunctionInputsAndOutputs.qll | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ql/src/semmle/go/dataflow/FunctionInputsAndOutputs.qll b/ql/src/semmle/go/dataflow/FunctionInputsAndOutputs.qll
index 9282532a2f7..4699ee20be0 100644
--- a/ql/src/semmle/go/dataflow/FunctionInputsAndOutputs.qll
+++ b/ql/src/semmle/go/dataflow/FunctionInputsAndOutputs.qll
@@ -226,8 +226,8 @@ private class OutParameter extends FunctionOutput, TOutParameter {
   }
 
   override string toString() {
-    index = -1 and result = "result"
+    index = -1 and result = "receiver"
     or
-    index >= 0 and result = "result " + index
+    index >= 0 and result = "parameter " + index
   }
-}
\ No newline at end of file
+}