Merge pull request github#17505 from owen-mc/go/inheritance-tests

Go: Add tests for model inheritance and fix bug in promoted methods

Author: owen-mc

go/ql/lib/change-notes/2024-09-24-incorrectly-promoted-methods.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* A method in the method set of an embedded field of a struct should not be promoted to the method set of the struct if the struct has a method with the same name. This was not being enforced, which meant that there were two methods with the same qualified name, and models were sometimes being applied when they shouldn't have been. This has now been fixed.

go/ql/lib/semmle/go/Types.qll

@@ -1011,7 +1011,7 @@ class NamedType extends @namedtype, CompositeType {
       s.hasOwnField(_, _, embedded, true) and
       // ensure `m` can be promoted
       not s.hasOwnField(_, m, _, _) and
-      not exists(Method m2 | m2.getReceiverType() = this and m2.getName() = m)
+      not exists(Method m2 | m2.getReceiverBaseType() = this and m2.getName() = m)
     |
       // If S contains an embedded field T, the method set of S includes promoted methods with receiver T
       result = embedded.getMethod(m)

go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/go.mod

@@ -0,0 +1,5 @@
+module semmle.go.Packages
+
+go 1.21
+
+require github.com/nonexistent/test v0.0.0-20200203000000-0000000000000

go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/mad_IEmbedI1_subtypes_true.expected

@@ -0,0 +1,3 @@
+testFailures
+invalidModelRow
+failures

go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/mad_IEmbedI1_subtypes_true.ext.yml

@@ -0,0 +1,16 @@
+extensions:
+  - addsTo:
+      pack: codeql/go-all
+      extensible: sourceModel
+    data:
+      - ["github.com/nonexistent/test", "IEmbedI1", True, "Source", "", "", "ReturnValue", "remote", "manual"]
+  - addsTo:
+      pack: codeql/go-all
+      extensible: summaryModel
+    data:
+      - ["github.com/nonexistent/test", "IEmbedI1", True, "Step", "", "", "Argument[0]", "ReturnValue", "value", "manual"]
+  - addsTo:
+      pack: codeql/go-all
+      extensible: sinkModel
+    data:
+      - ["github.com/nonexistent/test", "IEmbedI1", True, "Sink", "", "", "Argument[0]", "path-injection", "manual"]

go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/mad_IEmbedI1_subtypes_true.ql

@@ -0,0 +1,28 @@
+import go
+import semmle.go.dataflow.ExternalFlow
+import ModelValidation
+import semmle.go.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
+import TestUtilities.InlineExpectationsTest
+import MakeTest<FlowTest>
+
+module Config implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  predicate isSink(DataFlow::Node sink) { sink = any(FileSystemAccess fsa).getAPathArgument() }
+}
+
+module Flow = TaintTracking::Global<Config>;
+
+module FlowTest implements TestSig {
+  string getARelevantTag() { result = "IEmbedI1[t]" }
+
+  predicate hasActualResult(Location location, string element, string tag, string value) {
+    tag = "IEmbedI1[t]" and
+    exists(DataFlow::Node sink | Flow::flowTo(sink) |
+      sink.hasLocationInfo(location.getFile().getAbsolutePath(), location.getStartLine(),
+        location.getStartColumn(), location.getEndLine(), location.getEndColumn()) and
+      element = sink.toString() and
+      value = ""
+    )
+  }
+}

go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/mad_IEmbedI2_subtypes_true.expected

@@ -0,0 +1,3 @@
+testFailures
+invalidModelRow
+failures

go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/mad_IEmbedI2_subtypes_true.ext.yml

@@ -0,0 +1,16 @@
+extensions:
+  - addsTo:
+      pack: codeql/go-all
+      extensible: sourceModel
+    data:
+      - ["github.com/nonexistent/test", "IEmbedI2", True, "Source", "", "", "ReturnValue", "remote", "manual"]
+  - addsTo:
+      pack: codeql/go-all
+      extensible: summaryModel
+    data:
+      - ["github.com/nonexistent/test", "IEmbedI2", True, "Step", "", "", "Argument[0]", "ReturnValue", "value", "manual"]
+  - addsTo:
+      pack: codeql/go-all
+      extensible: sinkModel
+    data:
+      - ["github.com/nonexistent/test", "IEmbedI2", True, "Sink", "", "", "Argument[0]", "path-injection", "manual"]

go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/mad_IEmbedI2_subtypes_true.ql

@@ -0,0 +1,28 @@
+import go
+import semmle.go.dataflow.ExternalFlow
+import ModelValidation
+import semmle.go.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
+import TestUtilities.InlineExpectationsTest
+import MakeTest<FlowTest>
+
+module Config implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  predicate isSink(DataFlow::Node sink) { sink = any(FileSystemAccess fsa).getAPathArgument() }
+}
+
+module Flow = TaintTracking::Global<Config>;
+
+module FlowTest implements TestSig {
+  string getARelevantTag() { result = "IEmbedI2[t]" }
+
+  predicate hasActualResult(Location location, string element, string tag, string value) {
+    tag = "IEmbedI2[t]" and
+    exists(DataFlow::Node sink | Flow::flowTo(sink) |
+      sink.hasLocationInfo(location.getFile().getAbsolutePath(), location.getStartLine(),
+        location.getStartColumn(), location.getEndLine(), location.getEndColumn()) and
+      element = sink.toString() and
+      value = ""
+    )
+  }
+}

go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/mad_SEmbedI1_subtypes_true.expected

@@ -0,0 +1,3 @@
+testFailures
+invalidModelRow
+failures