Go: Allow data flow through varargs parameters #11732

owen-mc · 2022-12-19T02:42:06Z

Fixes https://github.com/github/codeql-go-team/issues/268

go/ql/lib/semmle/go/Expr.qll

smowton

Minor niggles, plus the bot is right about spelling

smowton · 2023-01-04T17:46:49Z

go/ql/lib/semmle/go/dataflow/FunctionInputsAndOutputs.qll

+      arg.asExpr() =
+        c.getCall().getImplicitVarargsArgument(index - c.getTarget().getNumParameter() + 1)
+    |
+      result.(DataFlow::PostUpdateNode).getPreUpdateNode() = arg


Since this is now identical to line 305, push the disjunction inside the exists at line 300

smowton · 2023-01-04T17:49:28Z

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

-    ArgumentNode() { this = getArgument(c, i) }
+    ArgumentNode() {
+      exists(CallExpr call | call = c.getCall() |
+        this = getArgument(c, i) and not this.asExpr() = call.getImplicitVarargsArgument(_)


Is and not this.asExpr() = call.getImplicitVarargsArgument(_) ever false? The implicit arg is wrapped into a slice, and therefore shouldn't be a direct argument per getArgument?

Yes, that piece of code can be false. There is some ambiguity about whether arguments to a variadic parameter (which aren't slices with ... after them) should be called arguments or not. Currently they are returned by getArgument on CallExpr and getArgument on CallNode, but they aren't given argument nodes. I admit this is confusing, and I'm happy to change it for the sake of clarity, though it's unclear exactly what to change it to.

owen-mc · 2023-04-20T16:07:56Z

I have fixed most of the test failures. I believe there are only two left now, which will need closer examination. It is possible that some of the changes I made in early commits are misguided, since the implementation has changed since then, so I intend to go through those in more detail before this is ready to review.

We don't want a post update node for the implicit varargs slice, and we do want one for each argument which is stored in the implicit varargs slice.

owen-mc · 2023-04-25T06:36:22Z

I have fixed the remaining test failures, and cleaned up the commit history so that it makes sense. This is ready for review. I should update the change note to acknowledge more changes that might affect user code, most notably the change to CallNode.getArgument and the new predicate CallNode.getSyntacticArgument which replicates the old behaviour. But maybe that is best done after this PR has been reviewed.

smowton · 2023-04-25T10:18:18Z

go/ql/lib/semmle/go/dataflow/FunctionInputsAndOutputs.qll

@@ -74,7 +74,17 @@ private class ParameterInput extends FunctionInput, TInParameter {

  override predicate isParameter(int i) { i = index }

-  override DataFlow::Node getEntryNode(DataFlow::CallNode c) { result = c.getArgument(index) }
+  override DataFlow::Node getEntryNode(DataFlow::CallNode c) {
+    result = getArgument(c, index) and


Intentionally adding method receivers through use of getArgument not c.getArgument?

No, that was a copy and paste error 😬

smowton · 2023-04-25T10:20:25Z

go/ql/lib/semmle/go/dataflow/FunctionInputsAndOutputs.qll

-  override DataFlow::Node getEntryNode(DataFlow::CallNode c) { result = c.getArgument(index) }
+  override DataFlow::Node getEntryNode(DataFlow::CallNode c) {
+    result = getArgument(c, index) and
+    not (


Can all this be replaced by getSyntacticArgument?

smowton · 2023-04-25T10:21:08Z

go/ql/lib/semmle/go/dataflow/FunctionInputsAndOutputs.qll

@@ -299,8 +309,13 @@ private class OutParameter extends FunctionOutput, TOutParameter {
  override DataFlow::Node getExitNode(DataFlow::CallNode c) {
    exists(DataFlow::Node arg |
      arg = getArgument(c, index) and
-      // exclude slices passed to varargs parameters using `...` calls
-      not (c.hasEllipsis() and index = c.getNumArgument() - 1)
+      not (


Same here, except apparently we want to exclude explicit varargs arrays, so syntactic argument except a ... expression?

smowton · 2023-04-25T10:22:33Z

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

@@ -10,6 +10,7 @@ private newtype TNode =
  MkInstructionNode(IR::Instruction insn) or
  MkSsaNode(SsaDefinition ssa) or
  MkGlobalFunctionNode(Function f) or
+  MkImplicitVarargsSlice(CallExpr c) { c.getTarget().isVariadic() and not c.hasEllipsis() } or


Pull out c.getTarget().isVariadic() and not c.hasEllipsis() as CallExpr.hasImplicitVarargs, and perhaps CallNode.hasImplicitVarargs?

smowton · 2023-04-25T10:36:40Z

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

@@ -519,12 +550,62 @@ module Public {
          )
    }

+    /**
+     * Gets the data flow node corresponding to an argument of this call, where


Suggested change

* Gets the data flow node corresponding to an argument of this call, where

* Gets a data flow node corresponding to an argument of this call, where

smowton · 2023-04-25T10:37:17Z

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

+     * For calls of the form `f(g())` where `g` has multiple results, the arguments of the call to
+     * `i` are the (implicit) element extraction nodes for the call to `g`.
+     *
+     * For calls to variadic functions without an ellipsis (`...`), there is a single argument of type


Suggested change

* For calls to variadic functions without an ellipsis (`...`), there is a single argument of type

* For calls to variadic functions without an ellipsis (`...`), there is a single argument of CodeQL type

I've reworded it in a different way

smowton · 2023-04-25T10:37:38Z

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

+     *
+     * For calls to variadic functions without an ellipsis (`...`), there is a single argument of type
+     * `ImplicitVarargsSlice` corresponding to the variadic parameter. This is in contrast to the member
+     * predicate `getArgument` on `CallExpr`, which gets the syntactic arguments.


Suggested change

* predicate `getArgument` on `CallExpr`, which gets the syntactic arguments.

* predicate `getArgument` on `CallExpr`, which gets the syntactic arguments, or similarly `CallNode.getSyntacticArgument`.

I've reworded it in a different way

smowton · 2023-04-25T10:49:47Z

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

+      exists(SignatureType t, int lastParamIndex |
+        t = this.getACalleeIncludingExternals().getType() and
+        lastParamIndex = t.getNumParameter() - 1
+      |
+        if
+          not this.hasEllipsis() and
+          t.isVariadic() and
+          i >= lastParamIndex
+        then
+          result.(ImplicitVarargsSlice).getCallNode() = this and
+          i = lastParamIndex
+        else result = this.getSyntacticArgument(i)
+      )


Suggested change

exists(SignatureType t, int lastParamIndex |

t = this.getACalleeIncludingExternals().getType() and

lastParamIndex = t.getNumParameter() - 1

|

if

not this.hasEllipsis() and

t.isVariadic() and

i >= lastParamIndex

then

result.(ImplicitVarargsSlice).getCallNode() = this and

i = lastParamIndex

else result = this.getSyntacticArgument(i)

)

result = this.getSyntacticArgument(i) and

not result = this.getImplicitVarargsArgument(_)

or

i = this.getACalleeIncludingExternals().getType().getNumParameter() - 1 and

result.(ImplicitVarargsSlice).getCallNode() = this

It now has one only result corresponding to a variadic parameter. If the argument is followed by an ellipsis then it is just the argument itself. Otherwise it is a ImplicitVarargsSlice node.

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

smowton

Looking good; there are real test failures though

smowton · 2023-04-27T11:02:17Z

go/ql/lib/semmle/go/Expr.qll

@@ -857,6 +857,24 @@ class CallExpr extends CallOrConversionExpr {
  /** Gets the number of argument expressions of this call. */
  int getNumArgument() { result = count(this.getAnArgument()) }

+  /** Holds if this call is has implicit variadic arguments. */


Suggested change

/** Holds if this call is has implicit variadic arguments. */

/** Holds if this call has implicit variadic arguments. */

smowton · 2023-04-27T11:04:10Z

go/ql/lib/semmle/go/dataflow/FunctionInputsAndOutputs.qll

+      (
+        arg = c.getSyntacticArgument(index) and
+        // exclude slices passed to varargs parameters using `...` calls
+        not (c.hasEllipsis() and index = c.getNumArgument() - 1)
+        or
+        arg = c.(DataFlow::MethodCallNode).getReceiver() and
+        index = -1
+      )


Suggested change

(

arg = c.getSyntacticArgument(index) and

// exclude slices passed to varargs parameters using `...` calls

not (c.hasEllipsis() and index = c.getNumArgument() - 1)

or

arg = c.(DataFlow::MethodCallNode).getReceiver() and

index = -1

)

arg = c.getSyntacticArgument(index) and

// exclude slices passed to varargs parameters using a `...` expression

not (c.hasEllipsis() and index = c.getNumArgument() - 1)

or

arg = c.(DataFlow::MethodCallNode).getReceiver() and

index = -1

smowton · 2023-04-27T11:06:19Z

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

+     * Returns a single `Node` corresponding to a variadic parameter. If there is no corresponding
+     * argument with an ellipsis (`...`), then it is a `ImplicitVarargsSlice`. This is in contrast
+     * to `getArgument` on `CallExpr`, which gets the syntactic arguments. Use
+     * `getSyntacticArgument` to get that behaviour.


Suggested change

* `getSyntacticArgument` to get that behaviour.

* `getSyntacticArgument` to get that behavior.

smowton · 2023-04-27T11:15:52Z

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

+     */
+    Node getArgument(int i) {
+      result = this.getSyntacticArgument(i) and
+      not result = this.getImplicitVarargsArgument(_)


Suggested change

not result = this.getImplicitVarargsArgument(_)

not (this.getTarget().isVariadic() and i >= this.getTarget().getNumParameter() - 1)

sorry, should have noticed this would lead to an odd result for an explicit varargs ... expression

Note that we should avoid using getTarget() as it doesn't exist when the called expr is a variable.

smowton · 2023-04-27T11:17:25Z

go/ql/test/experimental/frameworks/CleverGo/UntrustedSources.go

-				structParam246.Key,   // $ untrustedFlowSource
-				structParam246.Value, // $ untrustedFlowSource
+			sink(structParam246.Key)   // $ untrustedFlowSource
+			sink(structParam246.Value) // $ untrustedFlowSource
 			)


Suggested change

)

This avoids using `getTarget()`, so it works even when that doesn't exist (for example when calling a variable with function type).

smowton

Some small cleanups then LGTM!

smowton · 2023-05-09T17:17:46Z

go/ql/lib/change-notes/2023-04-25-data-flow-varargs-parameters.md

+---
+category: minorAnalysis
+---
+* Fixed data flow through variadic function parameters. The arguments corresponding to a variadic parameter are no longer returned by `CallNode.getArgument(int i)` and `CallNode.getAnArgument()`, and hence aren't `ArgumentNode`s. They now have one result, which is an `ImplicitVarargsSlice` node. For example, a call `f(a b, c)` to a function `f(T...)` is treated like `f([]T{a, b, c})`. The old behaviour is preserved by `CallNode.getSyntacticArgument(int i)` and `CallNode.getASyntacticArgument()`. `CallExpr.getArgument(int i)` and `CallExpr.getAnArgument()` are unchanged, and will still have three results in the example given.


Suggested change

* Fixed data flow through variadic function parameters. The arguments corresponding to a variadic parameter are no longer returned by `CallNode.getArgument(int i)` and `CallNode.getAnArgument()`, and hence aren't `ArgumentNode`s. They now have one result, which is an `ImplicitVarargsSlice` node. For example, a call `f(a b, c)` to a function `f(T...)` is treated like `f([]T{a, b, c})`. The old behaviour is preserved by `CallNode.getSyntacticArgument(int i)` and `CallNode.getASyntacticArgument()`. `CallExpr.getArgument(int i)` and `CallExpr.getAnArgument()` are unchanged, and will still have three results in the example given.

* Fixed data flow through variadic function parameters. The arguments corresponding to a variadic parameter are no longer returned by `CallNode.getArgument(int i)` and `CallNode.getAnArgument()`, and hence aren't `ArgumentNode`s. They now have one result, which is an `ImplicitVarargsSlice` node. For example, a call `f(a, b, c)` to a function `f(T...)` is treated like `f([]T{a, b, c})`. The old behaviour is preserved by `CallNode.getSyntacticArgument(int i)` and `CallNode.getASyntacticArgument()`. `CallExpr.getArgument(int i)` and `CallExpr.getAnArgument()` are unchanged, and will still have three results in the example given.

smowton · 2023-05-09T17:19:35Z

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

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


Suggested change

result = this.getImplicitVarargsArgument((n / 2))

result = this.getImplicitVarargsArgument(n / 2)

smowton · 2023-05-09T17:21:27Z

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

+    (
+      exists(Write w | w.writesElement(node2, _, node1))
+      or
+      node1 = node2.(ImplicitVarargsSlice).getCallNode().getImplicitVarargsArgument(_)


Add and use getAnImplicitVarargsArgument(), like most indexing getters

smowton · 2023-05-09T17:24:03Z

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

+        (
+          preupd instanceof ArgumentNode and not preupd instanceof ImplicitVarargsSlice
+          or
+          preupd = any(CallNode c).getImplicitVarargsArgument(_)


Suggested change

preupd = any(CallNode c).getImplicitVarargsArgument(_)

preupd = any(CallNode c).getAnImplicitVarargsArgument()

smowton · 2023-05-09T17:24:34Z

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

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


Suggested change

this = methodCall.getSyntacticArgument(_)

this = methodCall.getASyntacticArgument()

smowton · 2023-05-09T17:27:57Z

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

@@ -134,40 +134,44 @@ module NetHttp {
    result = call.getReceiver()
  }

-  private class ResponseBody extends Http::ResponseBody::Range, DataFlow::ArgumentNode {
+  private class ResponseBody extends Http::ResponseBody::Range, DataFlow::Node {


Suggested change

private class ResponseBody extends Http::ResponseBody::Range, DataFlow::Node {

private class ResponseBody extends Http::ResponseBody::Range {

Because that class already derives from Node

smowton · 2023-05-09T17:38:33Z

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

+      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")
+        )


Suggested change

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")

I appreciate it's more verbose, but it's less of an odd-looking constraint, and hopefully less of a temptation to the join-orderer too!

smowton · 2023-05-10T21:08:13Z

I'd feel free to ignore the weird internal failure on the macos job.

owen-mc requested a review from a team as a code owner December 19, 2022 02:42

github-actions bot added documentation Go labels Dec 19, 2022

github-advanced-security bot found potential problems Dec 19, 2022

View reviewed changes

go/ql/lib/semmle/go/Expr.qll Fixed Show fixed Hide fixed

owen-mc force-pushed the go/fix/model-data-flow-through-varargs branch 2 times, most recently from 02de827 to 79ad9ff Compare January 4, 2023 16:01

smowton reviewed Jan 4, 2023

View reviewed changes

owen-mc force-pushed the go/fix/model-data-flow-through-varargs branch from e410bcf to dd09cc0 Compare January 4, 2023 21:55

owen-mc marked this pull request as draft January 21, 2023 08:31

owen-mc force-pushed the go/fix/model-data-flow-through-varargs branch from 2b321be to a363128 Compare February 11, 2023 14:02

smowton mentioned this pull request Mar 28, 2023

Go: mass-convert taint-flow models to models-as-data format #12562

Closed

owen-mc force-pushed the go/fix/model-data-flow-through-varargs branch from f117d7a to 8f14cc9 Compare April 20, 2023 16:06

owen-mc added 2 commits April 25, 2023 07:33

Allow data flow through varargs parameters

73b712a

Update PostUpdateNodes for implicit varargs slices

3e73e02

We don't want a post update node for the implicit varargs slice, and we do want one for each argument which is stored in the implicit varargs slice.

owen-mc force-pushed the go/fix/model-data-flow-through-varargs branch from 8f14cc9 to 4e50d51 Compare April 25, 2023 06:33

owen-mc marked this pull request as ready for review April 25, 2023 06:34

smowton reviewed Apr 25, 2023

View reviewed changes

owen-mc added 2 commits April 26, 2023 14:35

Update CallNode.getArgument for implicit varargs

1e3d818

It now has one only result corresponding to a variadic parameter. If the argument is followed by an ellipsis then it is just the argument itself. Otherwise it is a ImplicitVarargsSlice node.

Update ParameterInput.getEntryNode for implicit varargs slices

39da26e

owen-mc force-pushed the go/fix/model-data-flow-through-varargs branch from 4e50d51 to fb61ff1 Compare April 27, 2023 09:32

github-advanced-security bot found potential problems Apr 27, 2023

View reviewed changes

go/ql/lib/semmle/go/dataflow/internal/DataFlowNodes.qll Fixed Show fixed Hide fixed

smowton reviewed Apr 27, 2023

View reviewed changes

owen-mc added 5 commits April 28, 2023 06:09

Update OutParameter.getExitNode for implicit varargs slices

17077f3

Accept intended test result changes

2d3fed9

use CallNode.getSyntacticArgument

bc0f903

Do not use variadic sink fn in tests

f2368a9

Formatted parameters always a variadic parameter

3f095db

owen-mc force-pushed the go/fix/model-data-flow-through-varargs branch from fb61ff1 to aa5cdd7 Compare April 28, 2023 05:10

owen-mc added 5 commits April 28, 2023 09:23

Add CallExpr.getCalleeType()

f3c1c53

This avoids using `getTarget()`, so it works even when that doesn't exist (for example when calling a variable with function type).

Add CallExpr.hasImplicitArgs()

b928f13

Use CallExpr.hasImplicitArgs()

52cc611

Accept review suggestions

c7c0a73

Remove ArgumentNode assumption

8415c4a

owen-mc force-pushed the go/fix/model-data-flow-through-varargs branch from aa5cdd7 to a59e7a3 Compare April 28, 2023 08:25

owen-mc mentioned this pull request Apr 28, 2023

Go: Add html/template functions as sanitisers for XSS queries #12944

Merged

smowton reviewed May 9, 2023

View reviewed changes

owen-mc added 2 commits May 10, 2023 13:50

Add change note

8f41ff3

address review comments

1c66564

owen-mc force-pushed the go/fix/model-data-flow-through-varargs branch from a59e7a3 to 1c66564 Compare May 10, 2023 13:05

smowton approved these changes May 10, 2023

View reviewed changes

owen-mc merged commit 270ba09 into github:main May 11, 2023

owen-mc deleted the go/fix/model-data-flow-through-varargs branch May 11, 2023 04:26

smowton mentioned this pull request May 23, 2023

Hotfix: Go: count passing to a vararg function as escaping #13250

Merged

owen-mc mentioned this pull request Jun 29, 2023

go SqlInjection ... Ellipsis #10979

Closed

owen-mc mentioned this pull request May 25, 2024

Go: Convert models for variadic functions to use models-as-data #16592

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Go: Allow data flow through varargs parameters #11732

Go: Allow data flow through varargs parameters #11732

owen-mc commented Dec 19, 2022 •

edited

Loading

smowton left a comment

smowton Jan 4, 2023

smowton Jan 4, 2023

owen-mc Jan 4, 2023 •

edited

Loading

owen-mc commented Apr 20, 2023

owen-mc commented Apr 25, 2023

smowton Apr 25, 2023

owen-mc Apr 25, 2023

smowton Apr 25, 2023

smowton Apr 25, 2023

smowton Apr 25, 2023

smowton Apr 25, 2023

smowton Apr 25, 2023

owen-mc Apr 27, 2023

smowton Apr 25, 2023

owen-mc Apr 27, 2023

smowton Apr 25, 2023

smowton left a comment

smowton Apr 27, 2023

smowton Apr 27, 2023

smowton Apr 27, 2023

smowton Apr 27, 2023

owen-mc Apr 27, 2023

smowton Apr 27, 2023

smowton left a comment

smowton May 9, 2023

smowton May 9, 2023

smowton May 9, 2023

smowton May 9, 2023

smowton May 9, 2023

smowton May 9, 2023

smowton May 9, 2023

smowton commented May 10, 2023

	* Gets the data flow node corresponding to an argument of this call, where
	* Gets a data flow node corresponding to an argument of this call, where

	* For calls to variadic functions without an ellipsis (`...`), there is a single argument of type
	* For calls to variadic functions without an ellipsis (`...`), there is a single argument of CodeQL type

	* predicate `getArgument` on `CallExpr`, which gets the syntactic arguments.
	* predicate `getArgument` on `CallExpr`, which gets the syntactic arguments, or similarly `CallNode.getSyntacticArgument`.

	/** Holds if this call is has implicit variadic arguments. */
	/** Holds if this call has implicit variadic arguments. */

	* `getSyntacticArgument` to get that behaviour.
	* `getSyntacticArgument` to get that behavior.

	not result = this.getImplicitVarargsArgument(_)
	not (this.getTarget().isVariadic() and i >= this.getTarget().getNumParameter() - 1)

	result = this.getImplicitVarargsArgument((n / 2))
	result = this.getImplicitVarargsArgument(n / 2)

	preupd = any(CallNode c).getImplicitVarargsArgument(_)
	preupd = any(CallNode c).getAnImplicitVarargsArgument()

	this = methodCall.getSyntacticArgument(_)
	this = methodCall.getASyntacticArgument()

	private class ResponseBody extends Http::ResponseBody::Range, DataFlow::Node {
	private class ResponseBody extends Http::ResponseBody::Range {

Go: Allow data flow through varargs parameters #11732

Go: Allow data flow through varargs parameters #11732

Conversation

owen-mc commented Dec 19, 2022 • edited Loading

smowton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

owen-mc Jan 4, 2023 • edited Loading

Choose a reason for hiding this comment

owen-mc commented Apr 20, 2023

owen-mc commented Apr 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smowton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smowton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smowton commented May 10, 2023

owen-mc commented Dec 19, 2022 •

edited

Loading

owen-mc Jan 4, 2023 •

edited

Loading