From 2df8c275e0d76a7ce4cacfb8fa07c3ed36ce3cf3 Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Fri, 19 Jun 2020 08:14:54 +0100 Subject: [PATCH] Fix field lookup in cyclic structs. --- change-notes/2020-06-19-cyclic-field-lookup.md | 2 ++ ql/src/semmle/go/Types.qll | 1 + .../library-tests/semmle/go/Types/Field_getPackage.expected | 1 + .../semmle/go/Types/Field_hasQualifiedName2.expected | 1 + .../semmle/go/Types/Field_hasQualifiedName3.expected | 1 + .../library-tests/semmle/go/Types/QualifiedNames.expected | 1 + ql/test/library-tests/semmle/go/Types/StructFields.expected | 1 + ql/test/library-tests/semmle/go/Types/Types.expected | 1 + ql/test/library-tests/semmle/go/Types/cyclic.go | 5 +++++ 9 files changed, 14 insertions(+) create mode 100644 change-notes/2020-06-19-cyclic-field-lookup.md create mode 100644 ql/test/library-tests/semmle/go/Types/cyclic.go diff --git a/change-notes/2020-06-19-cyclic-field-lookup.md b/change-notes/2020-06-19-cyclic-field-lookup.md new file mode 100644 index 00000000000..04901d5757f --- /dev/null +++ b/change-notes/2020-06-19-cyclic-field-lookup.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* A bug has been fixed that could cause the analysis not to terminate in the presence of cycles through embedded struct fields. \ No newline at end of file diff --git a/ql/src/semmle/go/Types.qll b/ql/src/semmle/go/Types.qll index 1b246e3e904..010c21641ad 100644 --- a/ql/src/semmle/go/Types.qll +++ b/ql/src/semmle/go/Types.qll @@ -389,6 +389,7 @@ class StructType extends @structtype, CompositeType { ) { hasOwnField(_, name, tp, isEmbedded) and depth = 0 and isMethod = false or + not hasOwnField(_, name, _, _) and exists(Type embedded | hasEmbeddedField(embedded, depth - 1) | embedded.getUnderlyingType().(StructType).hasOwnField(_, name, tp, isEmbedded) and isMethod = false diff --git a/ql/test/library-tests/semmle/go/Types/Field_getPackage.expected b/ql/test/library-tests/semmle/go/Types/Field_getPackage.expected index a8f42259f7a..32f53ed1470 100644 --- a/ql/test/library-tests/semmle/go/Types/Field_getPackage.expected +++ b/ql/test/library-tests/semmle/go/Types/Field_getPackage.expected @@ -1,3 +1,4 @@ +| cyclic.go:4:3:4:3 | s | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types | | pkg1/embedding.go:19:23:19:26 | base | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 | | pkg1/embedding.go:22:27:22:30 | base | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 | | pkg1/embedding.go:25:24:25:31 | embedder | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 | diff --git a/ql/test/library-tests/semmle/go/Types/Field_hasQualifiedName2.expected b/ql/test/library-tests/semmle/go/Types/Field_hasQualifiedName2.expected index 92dba057c92..319b906debd 100644 --- a/ql/test/library-tests/semmle/go/Types/Field_hasQualifiedName2.expected +++ b/ql/test/library-tests/semmle/go/Types/Field_hasQualifiedName2.expected @@ -1,3 +1,4 @@ +| cyclic.go:4:3:4:3 | s | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.s | s | | pkg1/embedding.go:19:23:19:26 | base | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.embedder | base | | pkg1/embedding.go:19:23:19:26 | base | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.embedder2 | base | | pkg1/embedding.go:19:23:19:26 | base | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.embedder3 | base | diff --git a/ql/test/library-tests/semmle/go/Types/Field_hasQualifiedName3.expected b/ql/test/library-tests/semmle/go/Types/Field_hasQualifiedName3.expected index 4df101eb01c..dea6ddebd7e 100644 --- a/ql/test/library-tests/semmle/go/Types/Field_hasQualifiedName3.expected +++ b/ql/test/library-tests/semmle/go/Types/Field_hasQualifiedName3.expected @@ -1,3 +1,4 @@ +| cyclic.go:4:3:4:3 | s | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types | s | s | | pkg1/embedding.go:19:23:19:26 | base | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 | embedder | base | | pkg1/embedding.go:19:23:19:26 | base | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 | embedder2 | base | | pkg1/embedding.go:19:23:19:26 | base | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 | embedder3 | base | diff --git a/ql/test/library-tests/semmle/go/Types/QualifiedNames.expected b/ql/test/library-tests/semmle/go/Types/QualifiedNames.expected index 2721d0e6bbc..5ad970cca32 100644 --- a/ql/test/library-tests/semmle/go/Types/QualifiedNames.expected +++ b/ql/test/library-tests/semmle/go/Types/QualifiedNames.expected @@ -19,3 +19,4 @@ | embedder3 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.embedder3 | | embedder4 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.embedder4 | | ptrembedder | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.ptrembedder | +| s | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.s | diff --git a/ql/test/library-tests/semmle/go/Types/StructFields.expected b/ql/test/library-tests/semmle/go/Types/StructFields.expected index 18363fb3e27..6a1e2c41d24 100644 --- a/ql/test/library-tests/semmle/go/Types/StructFields.expected +++ b/ql/test/library-tests/semmle/go/Types/StructFields.expected @@ -31,3 +31,4 @@ | embedder4 | pkg1/embedding.go:35:16:38:1 | struct type | base | base | | embedder4 | pkg1/embedding.go:35:16:38:1 | struct type | f | int | | ptrembedder | pkg1/embedding.go:22:18:22:32 | struct type | base | * base | +| s | cyclic.go:3:8:5:1 | struct type | s | * s | diff --git a/ql/test/library-tests/semmle/go/Types/Types.expected b/ql/test/library-tests/semmle/go/Types/Types.expected index 04cce22549d..2e85e97c95e 100644 --- a/ql/test/library-tests/semmle/go/Types/Types.expected +++ b/ql/test/library-tests/semmle/go/Types/Types.expected @@ -19,3 +19,4 @@ | embedder3 | embedder3 | | embedder4 | embedder4 | | ptrembedder | ptrembedder | +| s | s | diff --git a/ql/test/library-tests/semmle/go/Types/cyclic.go b/ql/test/library-tests/semmle/go/Types/cyclic.go new file mode 100644 index 00000000000..2e41e11cb3e --- /dev/null +++ b/ql/test/library-tests/semmle/go/Types/cyclic.go @@ -0,0 +1,5 @@ +package main + +type s struct { + *s +}