From 1218615a338a45c2e20fb8c67948af1bc593c2ac Mon Sep 17 00:00:00 2001 From: Keith Zantow Date: Mon, 28 Oct 2024 17:15:42 -0400 Subject: [PATCH] fix: malformed pom files cause infinite loop Signed-off-by: Keith Zantow --- .../cataloger/java/internal/maven/resolver.go | 15 ++++++---- .../java/internal/maven/resolver_test.go | 28 +++++++++++++++++++ .../test-fixtures/local/circular-1/pom.xml | 13 +++++++++ .../test-fixtures/local/circular-2/pom.xml | 13 +++++++++ .../my/org/circular/1.2.3/circular-1.2.3.pom | 13 +++++++++ 5 files changed, 77 insertions(+), 5 deletions(-) create mode 100644 syft/pkg/cataloger/java/internal/maven/test-fixtures/local/circular-1/pom.xml create mode 100644 syft/pkg/cataloger/java/internal/maven/test-fixtures/local/circular-2/pom.xml create mode 100644 syft/pkg/cataloger/java/internal/maven/test-fixtures/maven-repo/my/org/circular/1.2.3/circular-1.2.3.pom diff --git a/syft/pkg/cataloger/java/internal/maven/resolver.go b/syft/pkg/cataloger/java/internal/maven/resolver.go index 710600ae2f6..ffc406b35bd 100644 --- a/syft/pkg/cataloger/java/internal/maven/resolver.go +++ b/syft/pkg/cataloger/java/internal/maven/resolver.go @@ -135,9 +135,14 @@ func (r *Resolver) resolveProperty(ctx context.Context, resolutionContext []*Pro return value, nil } + var resolvingParents []*Project for _, pom := range resolutionContext { current := pom for parentDepth := 0; current != nil; parentDepth++ { + if slices.Contains(resolvingParents, current) { + log.WithFields("property", propertyExpression, "mavenID", r.resolveID(ctx, resolvingProperties, resolvingParents...)).Error("got circular reference while resolving property") + break // some sort of circular reference -- we've already seen this project + } if r.cfg.MaxParentRecursiveDepth > 0 && parentDepth > r.cfg.MaxParentRecursiveDepth { return "", fmt.Errorf("maximum parent recursive depth (%v) reached resolving property: %v", r.cfg.MaxParentRecursiveDepth, propertyExpression) } @@ -146,6 +151,7 @@ func (r *Resolver) resolveProperty(ctx context.Context, resolutionContext []*Pro return r.resolveExpression(ctx, resolutionContext, value, resolvingProperties) // property values can contain expressions } } + resolvingParents = append(resolvingParents, current) current, err = r.resolveParent(ctx, current, resolvingProperties...) if err != nil { return "", err @@ -271,13 +277,12 @@ func (r *Resolver) resolveID(ctx context.Context, resolvingProperties []string, artifactID := r.resolvePropertyValue(ctx, pom.ArtifactID, resolvingProperties, resolutionContext...) version := r.resolvePropertyValue(ctx, pom.Version, resolvingProperties, resolutionContext...) if pom.Parent != nil { - if groupID == "" { + // groupId and version are able to be inherited from the parent, but importantly: not artifactId. see: + // https://maven.apache.org/guides/introduction/introduction-to-the-pom.html#the-solution + if groupID == "" && deref(pom.GroupID) == "" { groupID = r.resolvePropertyValue(ctx, pom.Parent.GroupID, resolvingProperties, resolutionContext...) } - if artifactID == "" { - artifactID = r.resolvePropertyValue(ctx, pom.Parent.ArtifactID, resolvingProperties, resolutionContext...) - } - if version == "" { + if version == "" && deref(pom.Version) == "" { version = r.resolvePropertyValue(ctx, pom.Parent.Version, resolvingProperties, resolutionContext...) } } diff --git a/syft/pkg/cataloger/java/internal/maven/resolver_test.go b/syft/pkg/cataloger/java/internal/maven/resolver_test.go index f8c7e3d65c3..34042593a6d 100644 --- a/syft/pkg/cataloger/java/internal/maven/resolver_test.go +++ b/syft/pkg/cataloger/java/internal/maven/resolver_test.go @@ -243,6 +243,13 @@ func Test_mavenResolverRemote(t *testing.T) { expression: "${project.one}", expected: "1", }, + { + groupID: "my.org", // this particular package has a circular reference + artifactID: "circular", + version: "1.2.3", + expression: "${unresolved}", + expected: "", + }, } for _, test := range tests { @@ -312,6 +319,27 @@ func Test_relativePathParent(t *testing.T) { require.Equal(t, "", id.Version) }, }, + { + name: "circular resolving ID variables", + pom: "circular-1/pom.xml", + validate: func(t *testing.T, r *Resolver, pom *Project) { + require.NotNil(t, pom) + id := r.ResolveID(ctx, pom) + // version should be resolved, but not artifactId + require.Equal(t, "1.2.3", id.Version) + require.Equal(t, "", id.ArtifactID) + }, + }, + { + name: "circular parent only", + pom: "circular-2/pom.xml", + validate: func(t *testing.T, r *Resolver, pom *Project) { + require.NotNil(t, pom) + id := r.ResolveID(ctx, pom) + require.Equal(t, "", id.Version) + require.Equal(t, "something", id.ArtifactID) + }, + }, } for _, test := range tests { diff --git a/syft/pkg/cataloger/java/internal/maven/test-fixtures/local/circular-1/pom.xml b/syft/pkg/cataloger/java/internal/maven/test-fixtures/local/circular-1/pom.xml new file mode 100644 index 00000000000..2bfb3e73b56 --- /dev/null +++ b/syft/pkg/cataloger/java/internal/maven/test-fixtures/local/circular-1/pom.xml @@ -0,0 +1,13 @@ + + + 4.0.0 + + ${groupId} + ${artifactId} + + + my.org + circular + 1.2.3 + + diff --git a/syft/pkg/cataloger/java/internal/maven/test-fixtures/local/circular-2/pom.xml b/syft/pkg/cataloger/java/internal/maven/test-fixtures/local/circular-2/pom.xml new file mode 100644 index 00000000000..da5da368b33 --- /dev/null +++ b/syft/pkg/cataloger/java/internal/maven/test-fixtures/local/circular-2/pom.xml @@ -0,0 +1,13 @@ + + + 4.0.0 + + something + ${unresolvable} + + + my.org + circular + 1.2.3 + + diff --git a/syft/pkg/cataloger/java/internal/maven/test-fixtures/maven-repo/my/org/circular/1.2.3/circular-1.2.3.pom b/syft/pkg/cataloger/java/internal/maven/test-fixtures/maven-repo/my/org/circular/1.2.3/circular-1.2.3.pom new file mode 100644 index 00000000000..6d5d88eba05 --- /dev/null +++ b/syft/pkg/cataloger/java/internal/maven/test-fixtures/maven-repo/my/org/circular/1.2.3/circular-1.2.3.pom @@ -0,0 +1,13 @@ + + + 4.0.0 + + circular + 1.2.3 + + + my.org + circular + 1.2.3 + +