From 33481f1d84b4ae4cb12c52b7d6e5ffed1b332531 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 3 Jun 2025 12:00:42 +0100 Subject: [PATCH 1/2] Add failing test for redefined types --- .../go/dataflow/ExternalFlowInheritance/test_fields.go | 10 ++++++++++ .../dataflow/ExternalFlowInheritance/test_methods.go | 6 ++++++ .../vendor/github.com/nonexistent/test/stub.go | 4 ++++ 3 files changed, 20 insertions(+) diff --git a/go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/test_fields.go b/go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/test_fields.go index e58d937fae3c..67b9cf831c93 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/test_fields.go +++ b/go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/test_fields.go @@ -83,3 +83,13 @@ func TestFieldsSEmbedS1AndSEmbedS1(t test.SEmbedS1AndSEmbedS1) { a := t.SourceField t.SinkField = a // $ S1[t] ql_S1 } + +func TestFieldsRedefinedP1(t test.RedefinedP1) { + a := t.SourceField + t.SinkField = a // $ P1[f] P1[t] ql_P1 +} + +func TestFieldsRedefinedS1(t test.RedefinedS1) { + a := t.SourceField + t.SinkField = a // $ ql_S1 MISSING: S1[f] S1[t] +} diff --git a/go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/test_methods.go b/go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/test_methods.go index 8d4c592e0cd1..9063fc148d4a 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/test_methods.go +++ b/go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/test_methods.go @@ -171,3 +171,9 @@ func TestMethodsSEmbedS1AndSEmbedS1(t test.SEmbedS1AndSEmbedS1) { y := t.Step(x) t.Sink(y) // $ I1[t] S1[t] ql_S1 } + +func TestMethodsRedefinedI1(t test.RedefinedI1) { + x := t.Source() + y := t.Step(x) + t.Sink(y) // $ ql_I1 MISSING: I1[f] I1[t] SPURIOUS: ql_P1 ql_S1 +} diff --git a/go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/vendor/github.com/nonexistent/test/stub.go b/go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/vendor/github.com/nonexistent/test/stub.go index 663bb5fedde0..93d1536d0f32 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/vendor/github.com/nonexistent/test/stub.go +++ b/go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/vendor/github.com/nonexistent/test/stub.go @@ -265,3 +265,7 @@ type SEmbedS1AndSEmbedS1 struct { SEmbedS1 UniqueFieldSEmbedS1AndSEmbedS1 int // this only exists to make this struct type different from other struct types } + +type RedefinedP1 P1 +type RedefinedS1 S1 +type RedefinedI1 I1 From 3d1bdb17456fcd91488f7a736f781d049e45c59b Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 3 Jun 2025 12:35:29 +0100 Subject: [PATCH 2/2] Fix inheritance through redefined type --- .../lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll | 9 ++++++++- .../go/dataflow/ExternalFlowInheritance/test_fields.go | 2 +- .../go/dataflow/ExternalFlowInheritance/test_methods.go | 2 +- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll b/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll index 35887d076c03..67f18f6ed3f1 100644 --- a/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll +++ b/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll @@ -377,7 +377,8 @@ module SourceSinkInterpretationInput implements or n2.asExpr() = n.asInstruction().(IR::EvalImplicitDerefInstruction).getOperand() | - result = lookThroughPointerType(skipImplicitFieldReads(n2).getType()) + result = + maybeLookThroughNamedType(lookThroughPointerType(skipImplicitFieldReads(n2).getType())) ) } @@ -395,6 +396,12 @@ module SourceSinkInterpretationInput implements .getBaseInstruction() } + private DefinedType maybeLookThroughNamedType(DefinedType t) { + // We do not have information on what a defined type is defined to be, only + // the underlying type, so we have to be a bit imprecise. + result.getBaseType() = t.getBaseType() + } + /** Provides additional sink specification logic. */ bindingset[c] predicate interpretOutput(string c, InterpretNode mid, InterpretNode node) { diff --git a/go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/test_fields.go b/go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/test_fields.go index 67b9cf831c93..b95e5c6acd6e 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/test_fields.go +++ b/go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/test_fields.go @@ -91,5 +91,5 @@ func TestFieldsRedefinedP1(t test.RedefinedP1) { func TestFieldsRedefinedS1(t test.RedefinedS1) { a := t.SourceField - t.SinkField = a // $ ql_S1 MISSING: S1[f] S1[t] + t.SinkField = a // $ S1[f] S1[t] ql_S1 } diff --git a/go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/test_methods.go b/go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/test_methods.go index 9063fc148d4a..f57f62da7833 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/test_methods.go +++ b/go/ql/test/library-tests/semmle/go/dataflow/ExternalFlowInheritance/test_methods.go @@ -175,5 +175,5 @@ func TestMethodsSEmbedS1AndSEmbedS1(t test.SEmbedS1AndSEmbedS1) { func TestMethodsRedefinedI1(t test.RedefinedI1) { x := t.Source() y := t.Step(x) - t.Sink(y) // $ ql_I1 MISSING: I1[f] I1[t] SPURIOUS: ql_P1 ql_S1 + t.Sink(y) // $ I1[f] I1[t] ql_I1 SPURIOUS: ql_P1 ql_S1 }