From bf824cac0a55188f82b7de4fb84dc621f9dcfcd9 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 19 Nov 2024 15:57:31 +0000 Subject: [PATCH 1/2] Allow package-level variables in MaD --- go/ql/lib/semmle/go/dataflow/ExternalFlow.qll | 5 +++-- .../go/dataflow/internal/FlowSummaryImpl.qll | 18 ++++++++++++++++++ .../ExternalTaintFlow/completetest.ext.yml | 2 ++ .../dataflow/ExternalTaintFlow/sinks.expected | 1 + .../dataflow/ExternalTaintFlow/sinks.ext.yml | 1 + .../dataflow/ExternalTaintFlow/srcs.expected | 1 + .../go/dataflow/ExternalTaintFlow/srcs.ext.yml | 3 ++- .../go/dataflow/ExternalTaintFlow/test.go | 3 +++ .../vendor/github.com/nonexistent/test/stub.go | 3 +++ .../ExternalValueFlow/completetest.ext.yml | 2 ++ .../dataflow/ExternalValueFlow/sinks.expected | 1 + .../dataflow/ExternalValueFlow/sinks.ext.yml | 1 + .../dataflow/ExternalValueFlow/srcs.expected | 1 + .../go/dataflow/ExternalValueFlow/srcs.ext.yml | 3 ++- .../go/dataflow/ExternalValueFlow/test.go | 3 +++ .../vendor/github.com/nonexistent/test/stub.go | 3 +++ 16 files changed, 47 insertions(+), 4 deletions(-) diff --git a/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll b/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll index a59c7294e80..5ae7b6a7f0d 100644 --- a/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll +++ b/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll @@ -38,7 +38,8 @@ * first 6 columns, and the `output` column specifies how data leaves the * element selected by the first 6 columns. An `input` can be either "", * "Argument[n]", or "Argument[n1..n2]": - * - "": Selects a write to the selected element in case this is a field. + * - "": Selects a write to the selected element in case this is a field or + * package-level variable. * - "Argument[n]": Selects an argument in a call to the selected element. * The arguments are zero-indexed, and `receiver` specifies the receiver. * - "Argument[n1..n2]": Similar to "Argument[n]" but selects any argument @@ -47,7 +48,7 @@ * An `output` can be either "", "Argument[n]", "Argument[n1..n2]", "Parameter", * "Parameter[n]", "Parameter[n1..n2]", , "ReturnValue", "ReturnValue[n]", or * "ReturnValue[n1..n2]": - * - "": Selects a read of a selected field. + * - "": Selects a read of a selected field or package-level variable. * - "Argument[n]": Selects the post-update value of an argument in a call to the * selected element. That is, the value of the argument after the call returns. * The arguments are zero-indexed, and `receiver` specifies the receiver. diff --git a/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll b/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll index b82039a32fe..40c68ceb900 100644 --- a/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll +++ b/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll @@ -399,6 +399,13 @@ module SourceSinkInterpretationInput implements c = "" and pragma[only_bind_into](e) = getElementWithQualifier(frn.getField(), frn.getBase()) ) + or + // A package-scope (or universe-scope) variable + exists(Variable v | not v instanceof Field | + c = "" and + n.(DataFlow::ReadNode).reads(v) and + pragma[only_bind_into](e).asEntity() = v + ) ) } @@ -420,6 +427,17 @@ module SourceSinkInterpretationInput implements fw.writesField(base, f, node.asNode()) and pragma[only_bind_into](e) = getElementWithQualifier(f, base) ) + or + // A package-scope (or universe-scope) variable + exists(Node n, SourceOrSinkElement e, DataFlow::Write w, Variable v | + n = node.asNode() and + e = mid.asElement() and + not v instanceof Field + | + c = "" and + w.writes(v, n) and + pragma[only_bind_into](e).asEntity() = v + ) } } diff --git a/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/completetest.ext.yml b/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/completetest.ext.yml index 79bf9128ef5..d89a9e04e16 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/completetest.ext.yml +++ b/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/completetest.ext.yml @@ -35,10 +35,12 @@ extensions: pack: codeql/go-all extensible: sourceModel data: + - ["github.com/nonexistent/test", "", False, "SourceVariable", "", "", "", "qltest", "manual"] - ["github.com/nonexistent/test", "A", False, "Src1", "", "", "ReturnValue", "qltest", "manual"] - addsTo: pack: codeql/go-all extensible: sinkModel data: + - ["github.com/nonexistent/test", "", False, "SinkVariable", "", "", "", "qltest", "manual"] - ["github.com/nonexistent/test", "B", False, "Sink1", "", "", "Argument[0]", "qltest", "manual"] - ["github.com/nonexistent/test", "B", False, "SinkManyArgs", "", "", "Argument[0..2]", "qltest", "manual"] diff --git a/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/sinks.expected b/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/sinks.expected index fc9adff8942..755c3f82279 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/sinks.expected +++ b/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/sinks.expected @@ -43,3 +43,4 @@ invalidModelRow | test.go:199:17:199:20 | arg1 | qltest | | test.go:199:23:199:26 | arg2 | qltest | | test.go:199:29:199:32 | arg3 | qltest | +| test.go:202:22:202:25 | temp | qltest | diff --git a/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/sinks.ext.yml b/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/sinks.ext.yml index 426e094c00c..ec19b822a8c 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/sinks.ext.yml +++ b/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/sinks.ext.yml @@ -3,6 +3,7 @@ extensions: pack: codeql/go-all extensible: sinkModel data: + - ["github.com/nonexistent/test", "", False, "SinkVariable", "", "", "", "qltest", "manual"] - ["github.com/nonexistent/test", "B", False, "Sink1", "", "", "Argument[0]", "qltest", "manual"] - ["github.com/nonexistent/test", "B", False, "SinkMethod", "", "", "Argument[receiver]", "qltest", "manual"] - ["github.com/nonexistent/test", "B", False, "SinkManyArgs", "", "", "Argument[0..2]", "qltest", "manual"] diff --git a/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/srcs.expected b/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/srcs.expected index d63fedba3fd..bd1525a984b 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/srcs.expected +++ b/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/srcs.expected @@ -21,3 +21,4 @@ invalidModelRow | test.go:183:17:183:24 | call to Src1 | qltest | | test.go:187:24:187:31 | call to Src1 | qltest | | test.go:191:24:191:31 | call to Src1 | qltest | +| test.go:201:10:201:28 | selection of SourceVariable | qltest | diff --git a/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/srcs.ext.yml b/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/srcs.ext.yml index 5493650132c..cda2183894c 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/srcs.ext.yml +++ b/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/srcs.ext.yml @@ -3,9 +3,10 @@ extensions: pack: codeql/go-all extensible: sourceModel data: + - ["github.com/nonexistent/test", "", False, "SourceVariable", "", "", "", "qltest", "manual"] - ["github.com/nonexistent/test", "A", False, "Src1", "", "", "ReturnValue", "qltest", "manual"] - ["github.com/nonexistent/test", "A", False, "Src2", "", "", "ReturnValue", "qltest", "manual"] - ["github.com/nonexistent/test", "A", True, "Src2", "", "", "ReturnValue", "qltest-w-subtypes", "manual"] - ["github.com/nonexistent/test", "A", False, "SrcArg", "", "", "Argument[0]", "qltest-arg", "manual"] - ["github.com/nonexistent/test", "A", False, "Src3", "", "", "ReturnValue[0]", "qltest", "manual"] - - ["github.com/nonexistent/test", "A", True, "Src3", "", "", "ReturnValue[1]", "qltest-w-subtypes", "manual"] \ No newline at end of file + - ["github.com/nonexistent/test", "A", True, "Src3", "", "", "ReturnValue[1]", "qltest-w-subtypes", "manual"] diff --git a/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/test.go b/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/test.go index 33e980dac99..29ed066cd50 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/test.go +++ b/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/test.go @@ -197,6 +197,9 @@ func simpleflow() { arg3 := src arg4 := src b.SinkManyArgs(arg1, arg2, arg3, arg4) // $ hasTaintFlow="arg1" hasTaintFlow="arg2" hasTaintFlow="arg3" + + temp := test.SourceVariable + test.SinkVariable = temp // $ hasTaintFlow="temp" } type mapstringstringtype map[string]string diff --git a/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/vendor/github.com/nonexistent/test/stub.go b/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/vendor/github.com/nonexistent/test/stub.go index 05a5f741d76..72681cf7238 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/vendor/github.com/nonexistent/test/stub.go +++ b/go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/vendor/github.com/nonexistent/test/stub.go @@ -72,3 +72,6 @@ func (c C) Get() string { return "" } func (c *C) SetThroughPointer(f string) {} func (c *C) GetThroughPointer() string { return "" } + +var SourceVariable string +var SinkVariable string diff --git a/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/completetest.ext.yml b/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/completetest.ext.yml index 8fbc26ff6cd..924e19a8a73 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/completetest.ext.yml +++ b/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/completetest.ext.yml @@ -35,10 +35,12 @@ extensions: pack: codeql/go-all extensible: sourceModel data: + - ["github.com/nonexistent/test", "", False, "SourceVariable", "", "", "", "qltest", "manual"] - ["github.com/nonexistent/test", "A", False, "Src1", "", "", "ReturnValue", "qltest", "manual"] - addsTo: pack: codeql/go-all extensible: sinkModel data: + - ["github.com/nonexistent/test", "", False, "SinkVariable", "", "", "", "qltest", "manual"] - ["github.com/nonexistent/test", "B", False, "Sink1", "", "", "Argument[0]", "qltest", "manual"] - ["github.com/nonexistent/test", "B", False, "SinkManyArgs", "", "", "Argument[0..2]", "qltest", "manual"] diff --git a/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/sinks.expected b/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/sinks.expected index 0fe3a614e11..c9940e181c8 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/sinks.expected +++ b/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/sinks.expected @@ -49,3 +49,4 @@ invalidModelRow | test.go:205:10:205:26 | call to min | qltest | | test.go:206:10:206:26 | call to min | qltest | | test.go:207:10:207:26 | call to min | qltest | +| test.go:210:22:210:25 | temp | qltest | diff --git a/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/sinks.ext.yml b/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/sinks.ext.yml index 426e094c00c..ec19b822a8c 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/sinks.ext.yml +++ b/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/sinks.ext.yml @@ -3,6 +3,7 @@ extensions: pack: codeql/go-all extensible: sinkModel data: + - ["github.com/nonexistent/test", "", False, "SinkVariable", "", "", "", "qltest", "manual"] - ["github.com/nonexistent/test", "B", False, "Sink1", "", "", "Argument[0]", "qltest", "manual"] - ["github.com/nonexistent/test", "B", False, "SinkMethod", "", "", "Argument[receiver]", "qltest", "manual"] - ["github.com/nonexistent/test", "B", False, "SinkManyArgs", "", "", "Argument[0..2]", "qltest", "manual"] diff --git a/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/srcs.expected b/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/srcs.expected index d63fedba3fd..6fcfcc2a3bc 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/srcs.expected +++ b/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/srcs.expected @@ -21,3 +21,4 @@ invalidModelRow | test.go:183:17:183:24 | call to Src1 | qltest | | test.go:187:24:187:31 | call to Src1 | qltest | | test.go:191:24:191:31 | call to Src1 | qltest | +| test.go:209:10:209:28 | selection of SourceVariable | qltest | diff --git a/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/srcs.ext.yml b/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/srcs.ext.yml index 5493650132c..cda2183894c 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/srcs.ext.yml +++ b/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/srcs.ext.yml @@ -3,9 +3,10 @@ extensions: pack: codeql/go-all extensible: sourceModel data: + - ["github.com/nonexistent/test", "", False, "SourceVariable", "", "", "", "qltest", "manual"] - ["github.com/nonexistent/test", "A", False, "Src1", "", "", "ReturnValue", "qltest", "manual"] - ["github.com/nonexistent/test", "A", False, "Src2", "", "", "ReturnValue", "qltest", "manual"] - ["github.com/nonexistent/test", "A", True, "Src2", "", "", "ReturnValue", "qltest-w-subtypes", "manual"] - ["github.com/nonexistent/test", "A", False, "SrcArg", "", "", "Argument[0]", "qltest-arg", "manual"] - ["github.com/nonexistent/test", "A", False, "Src3", "", "", "ReturnValue[0]", "qltest", "manual"] - - ["github.com/nonexistent/test", "A", True, "Src3", "", "", "ReturnValue[1]", "qltest-w-subtypes", "manual"] \ No newline at end of file + - ["github.com/nonexistent/test", "A", True, "Src3", "", "", "ReturnValue[1]", "qltest-w-subtypes", "manual"] diff --git a/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/test.go b/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/test.go index 82419ae7d59..72c4db35248 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/test.go +++ b/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/test.go @@ -205,6 +205,9 @@ func simpleflow() { b.Sink1(min(srcInt, 0, 1)) // $ hasValueFlow="call to min" b.Sink1(min(0, srcInt, 1)) // $ hasValueFlow="call to min" b.Sink1(min(0, 1, srcInt)) // $ hasValueFlow="call to min" + + temp := test.SourceVariable + test.SinkVariable = temp // $ hasValueFlow="temp" } type mapstringstringtype map[string]string diff --git a/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/vendor/github.com/nonexistent/test/stub.go b/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/vendor/github.com/nonexistent/test/stub.go index 05a5f741d76..72681cf7238 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/vendor/github.com/nonexistent/test/stub.go +++ b/go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/vendor/github.com/nonexistent/test/stub.go @@ -72,3 +72,6 @@ func (c C) Get() string { return "" } func (c *C) SetThroughPointer(f string) {} func (c *C) GetThroughPointer() string { return "" } + +var SourceVariable string +var SinkVariable string From dd87b1a9de94c184612367fc4b349bc2e237ebcc Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 19 Nov 2024 15:20:31 +0000 Subject: [PATCH 2/2] Convert `os.stdin` model to MaD --- go/ql/lib/ext/os.model.yml | 1 + go/ql/lib/semmle/go/frameworks/stdlib/Os.qll | 8 -------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/go/ql/lib/ext/os.model.yml b/go/ql/lib/ext/os.model.yml index 3d87eefe43f..2c1c64db93a 100644 --- a/go/ql/lib/ext/os.model.yml +++ b/go/ql/lib/ext/os.model.yml @@ -53,6 +53,7 @@ extensions: - ["os", "", False, "Open", "", "", "ReturnValue[0]", "file", "manual"] - ["os", "", False, "OpenFile", "", "", "ReturnValue[0]", "file", "manual"] - ["os", "", False, "ReadFile", "", "", "ReturnValue[0]", "file", "manual"] + - ["os", "", False, "Stdin", "", "", "", "stdin", "manual"] - ["os", "", False, "UserCacheDir", "", "", "ReturnValue[0]", "environment", "manual"] - ["os", "", False, "UserConfigDir", "", "", "ReturnValue[0]", "environment", "manual"] - ["os", "", False, "UserHomeDir", "", "", "ReturnValue[0]", "environment", "manual"] diff --git a/go/ql/lib/semmle/go/frameworks/stdlib/Os.qll b/go/ql/lib/semmle/go/frameworks/stdlib/Os.qll index fb153451c59..72ea4cc6c57 100644 --- a/go/ql/lib/semmle/go/frameworks/stdlib/Os.qll +++ b/go/ql/lib/semmle/go/frameworks/stdlib/Os.qll @@ -43,12 +43,4 @@ module Os { input = inp and output = outp } } - - private class Stdin extends SourceNode { - Stdin() { - exists(Variable osStdin | osStdin.hasQualifiedName("os", "Stdin") | this = osStdin.getARead()) - } - - override string getThreatModel() { result = "stdin" } - } }