From d717734820c0a764a7fcfbb636bbea72fd019c73 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Fri, 3 Dec 2021 06:02:05 -0500 Subject: [PATCH] Do not allow "Argument" on its own --- ql/lib/semmle/go/dataflow/ExternalFlow.qll | 3 +-- .../semmle/go/dataflow/internal/FlowSummaryImpl.qll | 12 +++--------- .../semmle/go/dataflow/ExternalFlow/srcs.expected | 1 - .../semmle/go/dataflow/ExternalFlow/srcs.ql | 2 +- 4 files changed, 5 insertions(+), 13 deletions(-) diff --git a/ql/lib/semmle/go/dataflow/ExternalFlow.qll b/ql/lib/semmle/go/dataflow/ExternalFlow.qll index 48872777e82..24dd105f453 100644 --- a/ql/lib/semmle/go/dataflow/ExternalFlow.qll +++ b/ql/lib/semmle/go/dataflow/ExternalFlow.qll @@ -293,7 +293,6 @@ module CsvValidation { ( invalidSpecComponent(input, part) and not part = "" and - not (part = "Argument" and pred = "sink") and not parseArg(part, _) or specSplit(input, part, _) and @@ -309,7 +308,7 @@ module CsvValidation { | invalidSpecComponent(output, part) and not part = "" and - not (part = ["Argument", "Parameter"] and pred = "source") and + not (part = "Parameter" and pred = "source") and msg = "Unrecognized output specification \"" + part + "\" in " + pred + " model." ) or diff --git a/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll b/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll index bb6ab8867ea..e655419f454 100644 --- a/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll +++ b/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll @@ -721,13 +721,9 @@ module Private { not exists(interpretComponent(c)) } - private predicate inputNeedsReference(string c) { - c = "Argument" or - parseArg(c, _) - } + private predicate inputNeedsReference(string c) { parseArg(c, _) } private predicate outputNeedsReference(string c) { - c = "Argument" or parseArg(c, _) or c = "ReturnValue" or parseReturn(c, _) @@ -763,7 +759,7 @@ module Private { exists(int pos | node.asNode().(PostUpdateNode).getPreUpdateNode().(ArgNode).argumentOf(mid.asCall(), pos) | - c = "Argument" or parseArg(c, pos) + parseArg(c, pos) ) or exists(int pos | node.asNode().(ParamNode).isParameterOf(mid.asCallable(), pos) | @@ -791,9 +787,7 @@ module Private { interpretInput(input, idx + 1, ref, mid) and specSplit(input, c, idx) | - exists(int pos | node.asNode().(ArgNode).argumentOf(mid.asCall(), pos) | - c = "Argument" or parseArg(c, pos) - ) + exists(int pos | node.asNode().(ArgNode).argumentOf(mid.asCall(), pos) | parseArg(c, pos)) or exists(int pos, ReturnNodeExt ret | ( diff --git a/ql/test/library-tests/semmle/go/dataflow/ExternalFlow/srcs.expected b/ql/test/library-tests/semmle/go/dataflow/ExternalFlow/srcs.expected index 1bcc0daddac..a91ada0e749 100644 --- a/ql/test/library-tests/semmle/go/dataflow/ExternalFlow/srcs.expected +++ b/ql/test/library-tests/semmle/go/dataflow/ExternalFlow/srcs.expected @@ -1,7 +1,6 @@ invalidModelRow #select | test.go:12:6:12:8 | definition of arg | qltest-arg | -| test.go:37:6:37:6 | definition of a | qltest-arg | | test.go:40:8:40:15 | call to Src1 | qltest | | test.go:41:8:41:15 | call to Src2 | qltest | | test.go:41:8:41:15 | call to Src2 | qltest-w-subtypes | diff --git a/ql/test/library-tests/semmle/go/dataflow/ExternalFlow/srcs.ql b/ql/test/library-tests/semmle/go/dataflow/ExternalFlow/srcs.ql index 72e728271f9..4554effe729 100644 --- a/ql/test/library-tests/semmle/go/dataflow/ExternalFlow/srcs.ql +++ b/ql/test/library-tests/semmle/go/dataflow/ExternalFlow/srcs.ql @@ -11,7 +11,7 @@ class SourceModelTest extends SourceModelCsv { "github.com/nonexistent/test;A;false;Src1;;;ReturnValue;qltest", "github.com/nonexistent/test;A;false;Src2;;;ReturnValue;qltest", "github.com/nonexistent/test;A;true;Src2;;;ReturnValue;qltest-w-subtypes", - "github.com/nonexistent/test;A;false;SrcArg;;;Argument;qltest-arg", + "github.com/nonexistent/test;A;false;SrcArg;;;Argument[0];qltest-arg", "github.com/nonexistent/test;A;false;Src3;;;ReturnValue[0];qltest", "github.com/nonexistent/test;A;true;Src3;;;ReturnValue[1];qltest-w-subtypes" ]