address review comments

Author: owen-mc

go/ql/lib/semmle/go/StringOps.qll

@@ -331,7 +331,7 @@ module StringOps {
         formatDirective = this.getComponent(n) and
         formatDirective.charAt(0) = "%" and
         formatDirective.charAt(1) != "%" and
-        result = this.getImplicitVarargsArgument((n / 2))
+        result = this.getImplicitVarargsArgument(n / 2)
       }
     }
   }

go/ql/lib/semmle/go/dataflow/internal/ContainerFlow.qll

@@ -23,7 +23,7 @@ predicate containerStoreStep(Node node1, Node node2, Content c) {
     (
       exists(Write w | w.writesElement(node2, _, node1))
       or
-      node1 = node2.(ImplicitVarargsSlice).getCallNode().getImplicitVarargsArgument(_)
+      node1 = node2.(ImplicitVarargsSlice).getCallNode().getAnImplicitVarargsArgument()
     )
   )
   or

go/ql/lib/semmle/go/dataflow/internal/DataFlowNodes.qll

@@ -598,6 +598,12 @@ module Public {
       )
     }
 
+    /**
+     * Gets an argument without an ellipsis after it which is passed to
+     * the varargs parameter of the target of this call (if there is one).
+     */
+    Node getAnImplicitVarargsArgument() { result = this.getImplicitVarargsArgument(_) }
+
     /** Gets a function passed as the `i`th argument of this call. */
     FunctionNode getCallback(int i) { result.getASuccessor*() = this.getArgument(i) }
 
@@ -772,7 +778,7 @@ module Public {
         (
           preupd instanceof ArgumentNode and not preupd instanceof ImplicitVarargsSlice
           or
-          preupd = any(CallNode c).getImplicitVarargsArgument(_)
+          preupd = any(CallNode c).getAnImplicitVarargsArgument()
         ) and
         mutableType(preupd.getType())
       ) and

go/ql/lib/semmle/go/frameworks/Revel.qll

@@ -124,7 +124,7 @@ module Revel {
         or
         methodName = "RenderText" and
         contentType = "text/plain" and
-        this = methodCall.getSyntacticArgument(_)
+        this = methodCall.getASyntacticArgument()
       )
     }
 

go/ql/lib/semmle/go/frameworks/stdlib/NetHttp.qll

@@ -134,44 +134,43 @@ module NetHttp {
     result = call.getReceiver()
   }
 
-  private class ResponseBody extends Http::ResponseBody::Range, DataFlow::Node {
+  private class ResponseBody extends Http::ResponseBody::Range {
     DataFlow::Node responseWriter;
 
     ResponseBody() {
-      this = any(DataFlow::CallNode call).getASyntacticArgument() and
-      (
-        exists(DataFlow::CallNode call |
-          // A direct call to ResponseWriter.Write, conveying taint from the argument to the receiver
-          call.getTarget().(Method).implements("net/http", "ResponseWriter", "Write") and
-          this = call.getArgument(0) and
-          responseWriter = call.(DataFlow::MethodCallNode).getReceiver()
-        )
-        or
-        exists(TaintTracking::FunctionModel model |
-          // A modeled function conveying taint from some input to the response writer,
-          // e.g. `io.Copy(responseWriter, someTaintedReader)`
-          model.taintStep(this, responseWriter) and
-          responseWriter.getType().implements("net/http", "ResponseWriter")
-        )
-        or
-        exists(
-          SummarizedCallable callable, DataFlow::CallNode call, SummaryComponentStack input,
-          SummaryComponentStack output
-        |
-          callable = call.getACalleeIncludingExternals() and
-          callable.propagatesFlow(input, output, _)
-        |
-          // A modeled function conveying taint from some input to the response writer,
-          // e.g. `io.Copy(responseWriter, someTaintedReader)`
-          // NB. SummarizedCallables do not implement a direct call-site-crossing flow step; instead
-          // they are implemented by a function body with internal dataflow nodes, so we mimic the
-          // one-step style for the particular case of taint propagation direct from an argument or receiver
-          // to another argument, receiver or return value, matching the behavior for a `TaintTracking::FunctionModel`.
-          this = getSummaryInputOrOutputNode(call, input) and
-          responseWriter.(DataFlow::PostUpdateNode).getPreUpdateNode() =
-            getSummaryInputOrOutputNode(call, output) and
-          responseWriter.getType().implements("net/http", "ResponseWriter")
-        )
+      exists(DataFlow::CallNode call |
+        // A direct call to ResponseWriter.Write, conveying taint from the argument to the receiver
+        call.getTarget().(Method).implements("net/http", "ResponseWriter", "Write") and
+        this = call.getArgument(0) and
+        responseWriter = call.(DataFlow::MethodCallNode).getReceiver()
+      )
+      or
+      exists(TaintTracking::FunctionModel model |
+        // A modeled function conveying taint from some input to the response writer,
+        // e.g. `io.Copy(responseWriter, someTaintedReader)`
+        this = model.getACall().getASyntacticArgument() and
+        model.taintStep(this, responseWriter) and
+        responseWriter.getType().implements("net/http", "ResponseWriter")
+      )
+      or
+      exists(
+        SummarizedCallable callable, DataFlow::CallNode call, SummaryComponentStack input,
+        SummaryComponentStack output
+      |
+        this = call.getASyntacticArgument() and
+        callable = call.getACalleeIncludingExternals() and
+        callable.propagatesFlow(input, output, _)
+      |
+        // A modeled function conveying taint from some input to the response writer,
+        // e.g. `io.Copy(responseWriter, someTaintedReader)`
+        // NB. SummarizedCallables do not implement a direct call-site-crossing flow step; instead
+        // they are implemented by a function body with internal dataflow nodes, so we mimic the
+        // one-step style for the particular case of taint propagation direct from an argument or receiver
+        // to another argument, receiver or return value, matching the behavior for a `TaintTracking::FunctionModel`.
+        this = getSummaryInputOrOutputNode(call, input) and
+        responseWriter.(DataFlow::PostUpdateNode).getPreUpdateNode() =
+          getSummaryInputOrOutputNode(call, output) and
+        responseWriter.getType().implements("net/http", "ResponseWriter")
       )
     }
 

go/ql/src/experimental/frameworks/Fiber.qll

@@ -270,7 +270,7 @@ private module Fiber {
           or
           // signature: func (*Ctx) Send(bodies ...interface{})
           methodName = "Send" and
-          bodyNode = bodySetterCall.getSyntacticArgument(_)
+          bodyNode = bodySetterCall.getASyntacticArgument()
           or
           // signature: func (*Ctx) SendBytes(body []byte)
           methodName = "SendBytes" and
@@ -286,7 +286,7 @@ private module Fiber {
           or
           // signature: func (*Ctx) Write(bodies ...interface{})
           methodName = "Write" and
-          bodyNode = bodySetterCall.getSyntacticArgument(_)
+          bodyNode = bodySetterCall.getASyntacticArgument()
         )
       )
     )