From 8e481bdb7e817dea190626cf1313014516a5661a Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Wed, 22 Jan 2025 22:34:39 +0000 Subject: [PATCH 1/4] Add failing tests for "Parameter" in MaD --- .../semmle/go/dataflow/ExternalTaintFlow/completetest.ext.yml | 1 + .../semmle/go/dataflow/ExternalTaintFlow/sinks.expected | 1 + .../semmle/go/dataflow/ExternalTaintFlow/srcs.ext.yml | 1 + .../semmle/go/dataflow/ExternalTaintFlow/test.go | 4 ++++ 4 files changed, 7 insertions(+) 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 d89a9e04e16a..0ed8f80bbb60 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 @@ -37,6 +37,7 @@ extensions: data: - ["github.com/nonexistent/test", "", False, "SourceVariable", "", "", "", "qltest", "manual"] - ["github.com/nonexistent/test", "A", False, "Src1", "", "", "ReturnValue", "qltest", "manual"] + - ["semmle.go.Packages", "", True, "srcParam", "", "", "Parameter[0]", "qltest", "manual"] - addsTo: pack: codeql/go-all extensible: sinkModel 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 755c3f822791..b39135f827e3 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 @@ -44,3 +44,4 @@ invalidModelRow | test.go:199:23:199:26 | arg2 | qltest | | test.go:199:29:199:32 | arg3 | qltest | | test.go:202:22:202:25 | temp | qltest | +| test.go:206:10:206:12 | src | 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 cda2183894ca..ed050d27ec99 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 @@ -10,3 +10,4 @@ extensions: - ["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"] + - ["semmle.go.Packages", "", True, "srcParam", "", "", "Parameter[0]", "qltest", "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 29ed066cd50d..1a7ff2d78de7 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 @@ -202,6 +202,10 @@ func simpleflow() { test.SinkVariable = temp // $ hasTaintFlow="temp" } +func srcParam(src string, b test.B) { + b.Sink1(src) // $ MISSING: hasTaintFlow="src" +} + type mapstringstringtype map[string]string type arraytype []interface{} type channeltype chan interface{} From 08ea30ea8d703f343af27edffa231c707121ab88 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Wed, 22 Jan 2025 22:39:29 +0000 Subject: [PATCH 2/4] Fix bug in `InterpretNode.asCallable` It was only working for summarized callables. --- go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll | 4 +++- .../semmle/go/dataflow/ExternalTaintFlow/srcs.expected | 1 + .../semmle/go/dataflow/ExternalTaintFlow/test.go | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll b/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll index 62c19e1565d9..788b8e3ab662 100644 --- a/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll +++ b/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll @@ -252,7 +252,9 @@ module SourceSinkInterpretationInput implements /** Gets the callable that this node corresponds to, if any. */ DataFlowCallable asCallable() { - result.asSummarizedCallable().asFunction() = this.asElement().asEntity() + this.asElement().asEntity() = result.asSummarizedCallable().asFunction() or + this.asElement().asEntity() = result.asCallable().asFunction() or + this.asElement().asAstNode() = result.asCallable().asFuncLit() } /** Gets the target of this call, if any. */ 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 bd1525a984b9..a6f313198d1d 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 @@ -22,3 +22,4 @@ invalidModelRow | 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 | +| test.go:205:15:205:17 | definition of src | qltest | 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 1a7ff2d78de7..14b9a43b5991 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 @@ -203,7 +203,7 @@ func simpleflow() { } func srcParam(src string, b test.B) { - b.Sink1(src) // $ MISSING: hasTaintFlow="src" + b.Sink1(src) // $ hasTaintFlow="src" } type mapstringstringtype map[string]string From f055a78abf631e112676f8842cb5d7d712ae728d Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Wed, 22 Jan 2025 22:43:55 +0000 Subject: [PATCH 3/4] Copy "Parameter" tests to ExternalValueFlow --- .../semmle/go/dataflow/ExternalValueFlow/completetest.ext.yml | 1 + .../semmle/go/dataflow/ExternalValueFlow/sinks.expected | 1 + .../semmle/go/dataflow/ExternalValueFlow/srcs.expected | 1 + .../semmle/go/dataflow/ExternalValueFlow/srcs.ext.yml | 1 + .../semmle/go/dataflow/ExternalValueFlow/test.go | 4 ++++ 5 files changed, 8 insertions(+) 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 924e19a8a73b..bd1fcb365a07 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 @@ -37,6 +37,7 @@ extensions: data: - ["github.com/nonexistent/test", "", False, "SourceVariable", "", "", "", "qltest", "manual"] - ["github.com/nonexistent/test", "A", False, "Src1", "", "", "ReturnValue", "qltest", "manual"] + - ["semmle.go.Packages", "", True, "srcParam", "", "", "Parameter[0]", "qltest", "manual"] - addsTo: pack: codeql/go-all extensible: sinkModel 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 c9940e181c8c..e7421a9ad147 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 @@ -50,3 +50,4 @@ invalidModelRow | 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 | +| test.go:214:10:214:12 | src | qltest | 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 6fcfcc2a3bcc..a85e8689960f 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 @@ -22,3 +22,4 @@ invalidModelRow | 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 | +| test.go:213:15:213:17 | definition of src | 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 cda2183894ca..ed050d27ec99 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 @@ -10,3 +10,4 @@ extensions: - ["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"] + - ["semmle.go.Packages", "", True, "srcParam", "", "", "Parameter[0]", "qltest", "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 72c4db352480..f118880d4978 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 @@ -210,6 +210,10 @@ func simpleflow() { test.SinkVariable = temp // $ hasValueFlow="temp" } +func srcParam(src string, b test.B) { + b.Sink1(src) // $ hasValueFlow="src" +} + type mapstringstringtype map[string]string type arraytype []interface{} type channeltype chan interface{} From 577d9eb286ecad7a80f6da7d2c5eb2587d828288 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Wed, 22 Jan 2025 22:48:51 +0000 Subject: [PATCH 4/4] Add change note --- go/ql/lib/change-notes/2025-01-22-fix-parameter-in-models.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 go/ql/lib/change-notes/2025-01-22-fix-parameter-in-models.md diff --git a/go/ql/lib/change-notes/2025-01-22-fix-parameter-in-models.md b/go/ql/lib/change-notes/2025-01-22-fix-parameter-in-models.md new file mode 100644 index 000000000000..a14d31e89b1b --- /dev/null +++ b/go/ql/lib/change-notes/2025-01-22-fix-parameter-in-models.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Models-as-data models using "Parameter", "Parameter[n]" or "Parameter[n1..n2]" as the output now work correctly.