hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-24 08:15:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #11732 from owen-mc/go/fix/model-data-flow-through-varargs

Browse Source 
Go: Allow data flow through varargs parameters
This commit is contained in:
Owen Mansel-Chan
2023-05-11 05:26:40 +01:00
committed by GitHub
parent 619d25e32e 1c66564ccc
commit 270ba09ffb
34 changed files with 201 additions and 101 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
go/ql/lib/change-notes/2023-04-25-data-flow-varargs-parameters.md Normal file
View File

@@ -0,0 +1,4 @@
---
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.

27
go/ql/lib/semmle/go/Expr.qll
View File

@@ -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 has implicit variadic arguments. */
predicate hasImplicitVarargs() {
this.getCalleeType().isVariadic() and
not this.hasEllipsis()
}
/**
* Gets an argument with an ellipsis after it which is passed to a varargs
* parameter, as in `f(x...)`.
*
* Note that if the varargs parameter is `...T` then the type of the argument
* must be assignable to the slice type `[]T`.
*/
Expr getExplicitVarargsArgument() {
this.hasEllipsis() and
result = this.getArgument(this.getNumArgument() - 1)
}
/**
* Gets the name of the invoked function, method or variable if it can be
* determined syntactically.
@@ -873,6 +891,15 @@ class CallExpr extends CallOrConversionExpr {
)
}
/**
* Gets the signature type of the invoked function.
*
* Note that it avoids calling `getTarget()` so that it works even when that
* predicate isn't defined, for example when calling a variable with function
* type.
*/
SignatureType getCalleeType() { result = this.getCalleeExpr().getType() }
/** Gets the declared target of this call. */
Function getTarget() { this.getCalleeExpr() = result.getAReference() }

9
go/ql/lib/semmle/go/StringOps.qll
View File

@@ -219,7 +219,7 @@ module StringOps {
* replaced.
*/
DataFlow::Node getAReplacedArgument() {
exists(int n | n % 2 = 0 and result = this.getArgument(n))
exists(int n | n % 2 = 0 and result = this.getSyntacticArgument(n))
}
}
@@ -304,11 +304,6 @@ module StringOps {
* Gets the parameter index of the format string.
*/
abstract int getFormatStringIndex();
/**
* Gets the parameter index of the first parameter to be formatted.
*/
abstract int getFirstFormattedParameterIndex();
}
/**
@@ -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)
}
}
}

13
go/ql/lib/semmle/go/dataflow/FunctionInputsAndOutputs.qll
View File

@@ -74,7 +74,9 @@ 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 = c.getSyntacticArgument(index)
}
override DataFlow::Node getExitNode(FuncDef f) {
result = DataFlow::parameterNode(f.getParameter(index))
@@ -280,7 +282,7 @@ private class OutReceiver extends FunctionOutput, TOutReceiver {
/**
* A parameter of a function, viewed as an output.
*
* Note that slices passed to varargs parameters using `...` are not included, since in this
* Note that slices passed to variadic parameters using `...` are not included, since in this
* case it is ambiguous whether the output should be the slice itself or one of its elements.
*/
private class OutParameter extends FunctionOutput, TOutParameter {
@@ -298,9 +300,12 @@ 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
arg = c.getSyntacticArgument(index) and
// exclude slices followed by `...` passed to variadic parameters
not (c.hasEllipsis() and index = c.getNumArgument() - 1)
or
arg = c.(DataFlow::MethodCallNode).getReceiver() and
index = -1
|
result.(DataFlow::PostUpdateNode).getPreUpdateNode() = arg
)

8
go/ql/lib/semmle/go/dataflow/internal/ContainerFlow.qll
View File

@@ -10,7 +10,7 @@ private import semmle.go.dataflow.ExternalFlow
* Holds if the step from `node1` to `node2` stores a value in an array, a
* slice, a collection or a map. Thus, `node2` references an object with a
* content `c` that contains the value of `node1`. This covers array
* assignments and initializers as well as implicit array creations for
* assignments and initializers as well as implicit slice creations for
* varargs.
*/
predicate containerStoreStep(Node node1, Node node2, Content c) {
@@ -20,7 +20,11 @@ predicate containerStoreStep(Node node1, Node node2, Content c) {
node2.getType() instanceof ArrayType or
node2.getType() instanceof SliceType
) and
exists(Write w | w.writesElement(node2, _, node1))
(
exists(Write w | w.writesElement(node2, _, node1))
or
node1 = node2.(ImplicitVarargsSlice).getCallNode().getAnImplicitVarargsArgument()
)
)
or
c instanceof CollectionContent and

103
go/ql/lib/semmle/go/dataflow/internal/DataFlowNodes.qll
View File

@@ -10,6 +10,7 @@ private newtype TNode =
MkInstructionNode(IR::Instruction insn) or
MkSsaNode(SsaDefinition ssa) or
MkGlobalFunctionNode(Function f) or
MkImplicitVarargsSlice(CallExpr c) { c.hasImplicitVarargs() } or
MkSummarizedParameterNode(SummarizedCallable c, int i) {
FlowSummaryImpl::Private::summaryParameterNodeRange(c, i)
} or
@@ -426,6 +427,41 @@ module Public {
override ResultNode getAResult() { result.getRoot() = this.getExpr() }
}
/**
* An implicit varargs slice creation expression.
*
* A variadic function like `f(t1 T1, ..., Tm tm, A... x)` actually sees the
* varargs parameter as a slice `[]A`. A call `f(t1, ..., tm, x1, ..., xn)`
* desugars to `f(t1, ..., tm, []A{x1, ..., xn})`, and this node corresponds
* to this implicit slice creation.
*/
class ImplicitVarargsSlice extends Node, MkImplicitVarargsSlice {
CallNode call;
ImplicitVarargsSlice() { this = MkImplicitVarargsSlice(call.getCall()) }
override ControlFlow::Root getRoot() { result = call.getRoot() }
/** Gets the call containing this varargs slice creation argument. */
CallNode getCallNode() { result = call }
override Type getType() {
exists(Function f | f = call.getTarget() |
result = f.getParameterType(f.getNumParameter() - 1)
)
}
override string getNodeKind() { result = "implicit varargs slice" }
override string toString() { result = "[]type{args}" }
override predicate hasLocationInfo(
string filepath, int startline, int startcolumn, int endline, int endcolumn
) {
call.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
}
}
/**
* Gets a possible target of call `cn`.class
*
@@ -498,16 +534,11 @@ module Public {
CallExpr getCall() { result = this.getExpr() }
/**
* Gets the data flow node corresponding to the `i`th argument of this call.
*
* Note that the first argument in calls to the built-in function `make` is a type, which is
* not a data-flow node. It is skipped for the purposes of this predicate, so the (syntactically)
* second argument becomes the first argument in terms of data flow.
*
* 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`.
* Gets the `i`th argument of this call, where tuple extraction has been
* done but arguments corresponding to a variadic parameter are still
* considered separate.
*/
Node getArgument(int i) {
Node getSyntacticArgument(int i) {
if expr.getArgument(0).getType() instanceof TupleType
then result = DataFlow::extractTupleElement(DataFlow::exprNode(expr.getArgument(0)), i)
else
@@ -519,12 +550,60 @@ module Public {
)
}
/**
* Gets a data flow node corresponding to an argument of this call, where
* tuple extraction has been done but arguments corresponding to a variadic
* parameter are still considered separate.
*/
Node getASyntacticArgument() { result = this.getSyntacticArgument(_) }
/**
* Gets the data flow node corresponding to the `i`th argument of this call.
*
* Note that the first argument in calls to the built-in function `make` is a type, which is
* not a data-flow node. It is skipped for the purposes of this predicate, so the (syntactically)
* second argument becomes the first argument in terms of data flow.
*
* 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`.
*
* 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 behavior.
*/
Node getArgument(int i) {
result = this.getSyntacticArgument(i) and
not (expr.hasImplicitVarargs() and i >= expr.getCalleeType().getNumParameter() - 1)
or
i = expr.getCalleeType().getNumParameter() - 1 and
result.(ImplicitVarargsSlice).getCallNode() = this
}
/** Gets the data flow node corresponding to an argument of this call. */
Node getAnArgument() { result = this.getArgument(_) }
/** Gets the number of arguments of this call, if it can be determined. */
int getNumArgument() { result = count(this.getAnArgument()) }
/**
* Gets the 'i'th argument without an ellipsis after it which is passed to
* the varargs parameter of the target of this call (if there is one).
*/
Node getImplicitVarargsArgument(int i) {
i >= 0 and
expr.hasImplicitVarargs() and
exists(int lastParamIndex | lastParamIndex = expr.getCalleeType().getNumParameter() - 1 |
result = this.getSyntacticArgument(lastParamIndex + i)
)
}
/**
* 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) }
@@ -696,7 +775,11 @@ module Public {
or
preupd = getAWrittenNode()
or
preupd instanceof ArgumentNode and
(
preupd instanceof ArgumentNode and not preupd instanceof ImplicitVarargsSlice
or
preupd = any(CallNode c).getAnImplicitVarargsArgument()
) and
mutableType(preupd.getType())
) and
(

6
go/ql/lib/semmle/go/frameworks/Beego.qll
View File

@@ -253,7 +253,7 @@ module Beego {
this.getTarget().hasQualifiedName([packagePath(), logsPackagePath()], getALogFunctionName())
}
override DataFlow::Node getAMessageComponent() { result = this.getAnArgument() }
override DataFlow::Node getAMessageComponent() { result = this.getASyntacticArgument() }
}
private class BeegoLoggerMethods extends LoggerCall::Range, DataFlow::MethodCallNode {
@@ -261,13 +261,13 @@ module Beego {
this.getTarget().hasQualifiedName(logsPackagePath(), "BeeLogger", getALogFunctionName())
}
override DataFlow::Node getAMessageComponent() { result = this.getAnArgument() }
override DataFlow::Node getAMessageComponent() { result = this.getASyntacticArgument() }
}
private class UtilLoggers extends LoggerCall::Range, DataFlow::CallNode {
UtilLoggers() { this.getTarget().hasQualifiedName(utilsPackagePath(), "Display") }
override DataFlow::Node getAMessageComponent() { result = this.getAnArgument() }
override DataFlow::Node getAMessageComponent() { result = this.getASyntacticArgument() }
}
private class HtmlQuoteSanitizer extends SharedXss::Sanitizer {

2
go/ql/lib/semmle/go/frameworks/BeegoOrm.qll
View File

@@ -33,7 +33,7 @@ module BeegoOrm {
// Note this class doesn't do any escaping, unlike the true ORM part of the package
QueryBuilderSink() {
exists(Method impl | impl.implements(packagePath(), "QueryBuilder", _) |
this = impl.getACall().getAnArgument()
this = impl.getACall().getASyntacticArgument()
) and
this.getType().getUnderlyingType() instanceof StringType
}

6
go/ql/lib/semmle/go/frameworks/ElazarlGoproxy.qll
View File

@@ -90,7 +90,7 @@ module ElazarlGoproxy {
onreqcall.getTarget().hasQualifiedName(packagePath(), "ProxyHttpServer", "OnRequest")
|
handlerReg.getReceiver() = onreqcall.getASuccessor*() and
check = onreqcall.getArgument(0)
check = onreqcall.getSyntacticArgument(0)
)
}
}
@@ -112,13 +112,11 @@ module ElazarlGoproxy {
ProxyLogFunction() { this.hasQualifiedName(packagePath(), "ProxyCtx", ["Logf", "Warnf"]) }
override int getFormatStringIndex() { result = 0 }
override int getFirstFormattedParameterIndex() { result = 1 }
}
private class ProxyLog extends LoggerCall::Range, DataFlow::MethodCallNode {
ProxyLog() { this.getTarget() instanceof ProxyLogFunction }
override DataFlow::Node getAMessageComponent() { result = this.getAnArgument() }
override DataFlow::Node getAMessageComponent() { result = this.getASyntacticArgument() }
}
}

4
go/ql/lib/semmle/go/frameworks/Email.qll
View File

@@ -56,13 +56,13 @@ module EmailData {
// func NewV3MailInit(from *Email, subject string, to *Email, content ...*Content) *SGMailV3
exists(Function newv3MailInit |
newv3MailInit.hasQualifiedName(sendgridMail(), "NewV3MailInit") and
this = newv3MailInit.getACall().getArgument(any(int i | i = 1 or i >= 3))
this = newv3MailInit.getACall().getSyntacticArgument(any(int i | i = 1 or i >= 3))
)
or
// func (s *SGMailV3) AddContent(c ...*Content) *SGMailV3
exists(Method addContent |
addContent.hasQualifiedName(sendgridMail(), "SGMailV3", "AddContent") and
this = addContent.getACall().getAnArgument()
this = addContent.getACall().getASyntacticArgument()
)
}
}

4
go/ql/lib/semmle/go/frameworks/Glog.qll
View File

@@ -39,8 +39,6 @@ module Glog {
StringFormatter() { this.getName().matches("%f") }
override int getFormatStringIndex() { result = super.getFirstPrintedArg() }
override int getFirstFormattedParameterIndex() { result = super.getFirstPrintedArg() + 1 }
}
private class GlogCall extends LoggerCall::Range, DataFlow::CallNode {
@@ -49,7 +47,7 @@ module Glog {
GlogCall() { this = callee.getACall() }
override DataFlow::Node getAMessageComponent() {
result = this.getArgument(any(int i | i >= callee.getFirstPrintedArg()))
result = this.getSyntacticArgument(any(int i | i >= callee.getFirstPrintedArg()))
}
}
}

4
go/ql/lib/semmle/go/frameworks/Logrus.qll
View File

@@ -31,7 +31,7 @@ module Logrus {
private class LogCall extends LoggerCall::Range, DataFlow::CallNode {
LogCall() { this = any(LogFunction f).getACall() }
override DataFlow::Node getAMessageComponent() { result = this.getAnArgument() }
override DataFlow::Node getAMessageComponent() { result = this.getASyntacticArgument() }
}
private class StringFormatters extends StringOps::Formatting::Range instanceof LogFunction {
@@ -43,7 +43,5 @@ module Logrus {
}
override int getFormatStringIndex() { result = argOffset }
override int getFirstFormattedParameterIndex() { result = argOffset + 1 }
}
}

4
go/ql/lib/semmle/go/frameworks/Revel.qll
View File

@@ -124,7 +124,7 @@ module Revel {
or
methodName = "RenderText" and
contentType = "text/plain" and
this = methodCall.getAnArgument()
this = methodCall.getASyntacticArgument()
)
}
@@ -201,7 +201,7 @@ module Revel {
)
or
// a revel controller.Render(arg) will set controller.ViewArgs["arg"] = arg
exists(Variable arg | arg.getARead() = render.(ControllerRender).getAnArgument() |
exists(Variable arg | arg.getARead() = render.(ControllerRender).getASyntacticArgument() |
var.getBaseVariable() = arg and
var.getQualifiedName() = read.getFieldName()
)

4
go/ql/lib/semmle/go/frameworks/SQL.qll
View File

@@ -225,7 +225,7 @@ module SQL {
GormSink() {
exists(Method meth, string package, string name |
meth.hasQualifiedName(package, "DB", name) and
this = meth.getACall().getArgument(0) and
this = meth.getACall().getSyntacticArgument(0) and
package = Gorm::packagePath() and
name in [
"Where", "Raw", "Order", "Not", "Or", "Select", "Table", "Group", "Having", "Joins",
@@ -272,7 +272,7 @@ module Xorm {
XormSink() {
exists(Method meth, string type, string name, int n |
meth.hasQualifiedName(Xorm::packagePath(), type, name) and
this = meth.getACall().getArgument(n) and
this = meth.getACall().getSyntacticArgument(n) and
type = ["Engine", "Session"]
|
name =

4
go/ql/lib/semmle/go/frameworks/Spew.qll
View File

@@ -31,8 +31,6 @@ module Spew {
StringFormatter() { this.getName().matches("%f") }
override int getFormatStringIndex() { result = super.getFirstPrintedArg() }
override int getFirstFormattedParameterIndex() { result = super.getFirstPrintedArg() + 1 }
}
private class SpewCall extends LoggerCall::Range, DataFlow::CallNode {
@@ -41,7 +39,7 @@ module Spew {
SpewCall() { this = target.getACall() }
override DataFlow::Node getAMessageComponent() {
result = this.getArgument(any(int i | i >= target.getFirstPrintedArg()))
result = this.getSyntacticArgument(any(int i | i >= target.getFirstPrintedArg()))
}
}

13
go/ql/lib/semmle/go/frameworks/SystemCommandExecutors.qll
View File

@@ -14,11 +14,12 @@ private class ShellOrSudoExecution extends SystemCommandExecution::Range, DataFl
ShellOrSudoExecution() {
this instanceof SystemCommandExecution and
shellCommand = this.getAnArgument().getAPredecessor*() and
not hasSafeSubcommand(shellCommand.getStringValue(), this.getAnArgument().getStringValue())
shellCommand = this.getASyntacticArgument().getAPredecessor*() and
not hasSafeSubcommand(shellCommand.getStringValue(),
this.getASyntacticArgument().getStringValue())
}
override DataFlow::Node getCommandName() { result = this.getAnArgument() }
override DataFlow::Node getCommandName() { result = this.getASyntacticArgument() }
override predicate doubleDashIsSanitizing() {
shellCommand.getStringValue().matches("%" + ["git", "rsync"])
@@ -49,7 +50,7 @@ private class SystemCommandExecutors extends SystemCommandExecution::Range, Data
)
}
override DataFlow::Node getCommandName() { result = this.getArgument(cmdArg) }
override DataFlow::Node getCommandName() { result = this.getSyntacticArgument(cmdArg) }
}
/**
@@ -76,7 +77,7 @@ private class GoShCommandExecution extends SystemCommandExecution::Range, DataFl
)
}
override DataFlow::Node getCommandName() { result = this.getArgument(0) }
override DataFlow::Node getCommandName() { result = this.getSyntacticArgument(0) }
}
/**
@@ -102,7 +103,7 @@ module CryptoSsh {
)
}
override DataFlow::Node getCommandName() { result = this.getArgument(0) }
override DataFlow::Node getCommandName() { result = this.getSyntacticArgument(0) }
}
}

4
go/ql/lib/semmle/go/frameworks/Zap.qll
View File

@@ -32,8 +32,6 @@ module Zap {
ZapFormatter() { this.getName().matches("%f") }
override int getFormatStringIndex() { result = 0 }
override int getFirstFormattedParameterIndex() { result = 1 }
}
/**
@@ -45,7 +43,7 @@ module Zap {
private class ZapCall extends LoggerCall::Range, DataFlow::MethodCallNode {
ZapCall() { this = any(ZapFunction f).getACall() }
override DataFlow::Node getAMessageComponent() { result = this.getAnArgument() }
override DataFlow::Node getAMessageComponent() { result = this.getASyntacticArgument() }
}
// These are expressed using TaintTracking::FunctionModel because varargs functions don't work with Models-as-Data sumamries yet.

4
go/ql/lib/semmle/go/frameworks/stdlib/Fmt.qll
View File

@@ -30,7 +30,7 @@ module Fmt {
private class PrintCall extends LoggerCall::Range, DataFlow::CallNode {
PrintCall() { this.getTarget() instanceof Printer }
override DataFlow::Node getAMessageComponent() { result = this.getAnArgument() }
override DataFlow::Node getAMessageComponent() { result = this.getASyntacticArgument() }
}
/** The `Fprint` function or one of its variants. */
@@ -66,8 +66,6 @@ module Fmt {
}
override int getFormatStringIndex() { result = argOffset }
override int getFirstFormattedParameterIndex() { result = argOffset + 1 }
}
/** The `Sscan` function or one of its variants. */

4
go/ql/lib/semmle/go/frameworks/stdlib/Log.qll
View File

@@ -20,14 +20,12 @@ module Log {
LogFormatter() { this.getName().matches("%f") }
override int getFormatStringIndex() { result = 0 }
override int getFirstFormattedParameterIndex() { result = 1 }
}
private class LogCall extends LoggerCall::Range, DataFlow::CallNode {
LogCall() { this = any(LogFunction f).getACall() }
override DataFlow::Node getAMessageComponent() { result = this.getAnArgument() }
override DataFlow::Node getAMessageComponent() { result = this.getASyntacticArgument() }
}
/** A fatal log function, which calls `os.Exit`. */

7
go/ql/lib/semmle/go/frameworks/stdlib/NetHttp.qll
View File

@@ -134,7 +134,7 @@ module NetHttp {
result = call.getReceiver()
}
private class ResponseBody extends Http::ResponseBody::Range, DataFlow::ArgumentNode {
private class ResponseBody extends Http::ResponseBody::Range {
DataFlow::Node responseWriter;
ResponseBody() {
@@ -148,6 +148,7 @@ module NetHttp {
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")
)
@@ -156,7 +157,9 @@ module NetHttp {
SummarizedCallable callable, DataFlow::CallNode call, SummaryComponentStack input,
SummaryComponentStack output
|
callable = call.getACalleeIncludingExternals() and callable.propagatesFlow(input, 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)`

6
go/ql/lib/semmle/go/security/CommandInjection.qll
View File

@@ -47,7 +47,7 @@ module CommandInjection {
exists(DataFlow::CallNode c |
this = c and
(c = Builtin::append().getACall() or c = any(SystemCommandExecution sce)) and
c.getArgument(doubleDashIndex).getStringValue() = "--"
c.getSyntacticArgument(doubleDashIndex).getStringValue() = "--"
)
or
// array/slice literal containing a "--"
@@ -63,7 +63,7 @@ module CommandInjection {
alreadyHasDoubleDash.getType() instanceof SliceType
) and
this = userCall and
DataFlow::localFlow(alreadyHasDoubleDash, userCall.getArgument(doubleDashIndex))
DataFlow::localFlow(alreadyHasDoubleDash, userCall.getSyntacticArgument(doubleDashIndex))
)
}
@@ -71,7 +71,7 @@ module CommandInjection {
exists(int sanitizedIndex |
sanitizedIndex > doubleDashIndex and
(
result = this.(DataFlow::CallNode).getArgument(sanitizedIndex) or
result = this.(DataFlow::CallNode).getSyntacticArgument(sanitizedIndex) or
result = DataFlow::exprNode(this.asExpr().(ArrayOrSliceLit).getElement(sanitizedIndex))
)
)

4
go/ql/lib/semmle/go/security/Xss.qll
View File

@@ -73,12 +73,12 @@ module SharedXss {
exists(body.getAContentTypeNode())
or
exists(DataFlow::CallNode call | call.getTarget().hasQualifiedName("fmt", "Fprintf") |
body = call.getAnArgument() and
body = call.getASyntacticArgument() and
// checks that the format value does not start with (ignoring whitespace as defined by
// https://mimesniff.spec.whatwg.org/#whitespace-byte):
// - '<', which could lead to an HTML content type being detected, or
// - '%', which could be a format string.
call.getArgument(1).getStringValue().regexpMatch("(?s)[\\t\\n\\x0c\\r ]*+[^<%].*")
call.getSyntacticArgument(1).getStringValue().regexpMatch("(?s)[\\t\\n\\x0c\\r ]*+[^<%].*")
)
or
exists(DataFlow::Node pred | body = pred.getASuccessor*() |

2
go/ql/src/Security/CWE-352/ConstantOauth2State.ql
View File

@@ -109,7 +109,7 @@ class PrivateUrlFlowsToAuthCodeUrlCall extends DataFlow::Configuration {
exists(DataFlow::CallNode cn |
cn.getACalleeIncludingExternals().asFunction() instanceof Fmt::AppenderOrSprinter
|
pred = cn.getAnArgument() and succ = cn.getResult()
pred = cn.getASyntacticArgument() and succ = cn.getResult()
)
}

2
go/ql/src/Security/CWE-601/BadRedirectCheck.ql
View File

@@ -121,7 +121,7 @@ class Configuration extends TaintTracking::Configuration {
)
or
exists(DataFlow::CallNode call, int i | call.getTarget().hasQualifiedName("path", "Join") |
i > 0 and node = call.getArgument(i)
i > 0 and node = call.getSyntacticArgument(i)
)
}

2
go/ql/src/experimental/frameworks/CleverGo.qll
View File

@@ -278,7 +278,7 @@ private module CleverGo {
or
// signature: func (*Context) Stringf(code int, format string, a ...interface{}) error
methodName = "Stringf" and
bodyNode = bodySetterCall.getArgument([1, any(int i | i >= 2)]) and
bodyNode = bodySetterCall.getSyntacticArgument([1, any(int i | i >= 2)]) and
contentTypeString = "text/plain"
or
// signature: func (*Context) XML(code int, data interface{}) error

6
go/ql/src/experimental/frameworks/Fiber.qll
View File

@@ -183,7 +183,7 @@ private module Fiber {
// signature: func (*Ctx) Append(field string, values ...string)
methodName = "Append" and
headerNameNode = headerSetterCall.getArgument(0) and
headerValueNode = headerSetterCall.getArgument(any(int i | i >= 1))
headerValueNode = headerSetterCall.getSyntacticArgument(any(int i | i >= 1))
or
// signature: func (*Ctx) Set(key string, val string)
methodName = "Set" and
@@ -270,7 +270,7 @@ private module Fiber {
or
// signature: func (*Ctx) Send(bodies ...interface{})
methodName = "Send" and
bodyNode = bodySetterCall.getArgument(_)
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.getArgument(_)
bodyNode = bodySetterCall.getASyntacticArgument()
)
)
)

12
go/ql/test/experimental/frameworks/CleverGo/UntrustedSources.go
View File

@@ -14,10 +14,8 @@ func UntrustedSources_ClevergoTechClevergoV052() {
{
var receiverContext656 clevergo.Context
resultUsername414, resultPassword518, _ := receiverContext656.BasicAuth()
sink(
resultUsername414, // $ untrustedFlowSource
resultPassword518, // $ untrustedFlowSource
)
sink(resultUsername414) // $ untrustedFlowSource
sink(resultPassword518) // $ untrustedFlowSource
}
// func (*Context).Decode(v interface{}) (err error)
{
@@ -102,10 +100,8 @@ func UntrustedSources_ClevergoTechClevergoV052() {
// Untrusted flow sources from clevergo.tech/clevergo.Param struct fields.
{
structParam246 := new(clevergo.Param)
sink(
structParam246.Key, // $ untrustedFlowSource
structParam246.Value, // $ untrustedFlowSource
)
sink(structParam246.Key) // $ untrustedFlowSource
sink(structParam246.Value) // $ untrustedFlowSource
}
}
// Untrusted flow sources from types.

2
go/ql/test/experimental/frameworks/CleverGo/stubs.go
View File

@@ -7,6 +7,6 @@ func source() interface{} {
return nil
}
func sink(v ...interface{}) {}
func sink(v interface{}) {}
func link(from interface{}, into interface{}) {}

12
go/ql/test/experimental/frameworks/Fiber/UntrustedFlowSources.go
View File

@@ -121,13 +121,11 @@ func UntrustedFlowSources_GithubComGofiberFiberV1146() {
// Untrusted flow sources from github.com/gofiber/fiber.Cookie struct fields.
{
structCookie322 := new(fiber.Cookie)
sink(
structCookie322.Domain, // $ untrustedFlowSource
structCookie322.Name, // $ untrustedFlowSource
structCookie322.Path, // $ untrustedFlowSource
structCookie322.SameSite, // $ untrustedFlowSource
structCookie322.Value, // $ untrustedFlowSource
)
sink(structCookie322.Domain) // $ untrustedFlowSource
sink(structCookie322.Name) // $ untrustedFlowSource
sink(structCookie322.Path) // $ untrustedFlowSource
sink(structCookie322.SameSite) // $ untrustedFlowSource
sink(structCookie322.Value) // $ untrustedFlowSource
}
// Untrusted flow sources from github.com/gofiber/fiber.Error struct fields.
{

2
go/ql/test/experimental/frameworks/Fiber/stubs.go
View File

@@ -7,6 +7,6 @@ func source() interface{} {
return nil
}
func sink(v ...interface{}) {}
func sink(v interface{}) {}
func link(from interface{}, into interface{}) {}

4
go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowVarArgs/main.go
View File

@@ -24,7 +24,7 @@ func main() {
sink(test.FunctionWithParameter(sSlice[1])) // $ taintflow dataflow
sink(test.FunctionWithSliceParameter(sSlice)) // $ taintflow dataflow
sink(test.FunctionWithVarArgsParameter(sSlice...)) // $ taintflow dataflow
sink(test.FunctionWithVarArgsParameter(s0, s1)) // $ MISSING: taintflow dataflow
sink(test.FunctionWithVarArgsParameter(s0, s1)) // $ taintflow dataflow
sliceOfStructs := []test.A{{Field: source()}}
sink(sliceOfStructs[0].Field) // $ taintflow dataflow
@@ -34,5 +34,5 @@ func main() {
aSlice := []test.A{a0, a1}
sink(test.FunctionWithSliceOfStructsParameter(aSlice)) // $ taintflow dataflow
sink(test.FunctionWithVarArgsOfStructsParameter(aSlice...)) // $ taintflow dataflow
sink(test.FunctionWithVarArgsOfStructsParameter(a0, a1)) // $ MISSING: taintflow dataflow
sink(test.FunctionWithVarArgsOfStructsParameter(a0, a1)) // $ taintflow dataflow
}

8
go/ql/test/library-tests/semmle/go/dataflow/Nodes/CallNode_getArgument.expected
View File

@@ -1,6 +1,4 @@
| main.go:7:2:7:25 | call to Println | 0 | main.go:7:14:7:24 | ...+... |
| main.go:10:2:10:19 | call to Println | 0 | main.go:10:14:10:18 | ...+... |
| main.go:7:2:7:25 | call to Println | 0 | main.go:7:2:7:25 | []type{args} |
| main.go:10:2:10:19 | call to Println | 0 | main.go:10:2:10:19 | []type{args} |
| main.go:14:8:14:24 | call to make | 0 | main.go:14:23:14:23 | 1 |
| main.go:16:2:16:26 | call to Println | 0 | main.go:16:14:16:15 | ss |
| main.go:16:2:16:26 | call to Println | 1 | main.go:16:18:16:18 | 0 |
| main.go:16:2:16:26 | call to Println | 2 | main.go:16:21:16:25 | index expression |
| main.go:16:2:16:26 | call to Println | 0 | main.go:16:2:16:26 | []type{args} |

4
go/ql/test/library-tests/semmle/go/dataflow/VarArgs/main.go
View File

@@ -36,7 +36,7 @@ func main() {
sSlice := []string{s0, s1}
sink(functionWithSliceParameter(sSlice)) // $ taintflow dataflow
sink(functionWithVarArgsParameter(sSlice...)) // $ taintflow dataflow
sink(functionWithVarArgsParameter(s0, s1)) // $ MISSING: taintflow dataflow
sink(functionWithVarArgsParameter(s0, s1)) // $ taintflow dataflow
sliceOfStructs := []A{{f: source()}}
sink(sliceOfStructs[0].f) // $ taintflow dataflow
@@ -46,5 +46,5 @@ func main() {
aSlice := []A{a0, a1}
sink(functionWithSliceOfStructsParameter(aSlice)) // $ taintflow dataflow
sink(functionWithVarArgsOfStructsParameter(aSlice...)) // $ taintflow dataflow
sink(functionWithVarArgsOfStructsParameter(a0, a1)) // $ MISSING: taintflow dataflow
sink(functionWithVarArgsOfStructsParameter(a0, a1)) // $ taintflow dataflow
}

2
go/ql/test/library-tests/semmle/go/frameworks/SQL/Gorm/gorm.ql
View File

@@ -1,5 +1,5 @@
import go
from SQL::QueryString qs, Method meth, string a, string b, string c
where meth.hasQualifiedName(a, b, c) and qs = meth.getACall().getArgument(0)
where meth.hasQualifiedName(a, b, c) and qs = meth.getACall().getSyntacticArgument(0)
select qs, a, b, c