From b55c60cbd210d46e8d482d65ead4145c89b34365 Mon Sep 17 00:00:00 2001 From: Parth Patel <88045217+pxp928@users.noreply.github.com> Date: Tue, 12 Dec 2023 14:40:41 -0500 Subject: [PATCH] fix vulns query not properly evaluating isDependency (#1582) Signed-off-by: pxp928 --- cmd/guacone/cmd/vulnerability.go | 152 ++++++++++++++--------------- pkg/assembler/helpers/purl_test.go | 3 + 2 files changed, 78 insertions(+), 77 deletions(-) diff --git a/cmd/guacone/cmd/vulnerability.go b/cmd/guacone/cmd/vulnerability.go index e0740cc7d3..d1f0b56ed5 100644 --- a/cmd/guacone/cmd/vulnerability.go +++ b/cmd/guacone/cmd/vulnerability.go @@ -236,6 +236,7 @@ func vexSubjectIds(s model.AllCertifyVEXStatementSubjectPackageOrArtifact) []str func searchDependencyPackages(ctx context.Context, gqlclient graphql.Client, topPkgID string, maxLength int) ([]string, []table.Row, error) { var path []string var tableRows []table.Row + queue := make([]string, 0) // the queue of nodes in bfs type dfsNode struct { expanded bool // true once all node neighbors are added to queue @@ -266,47 +267,42 @@ func searchDependencyPackages(ctx context.Context, gqlclient graphql.Client, top continue } - depPkgFilter := &model.PkgSpec{ - Type: &isDependency.DependencyPackage.Type, - Namespace: &isDependency.DependencyPackage.Namespaces[0].Namespace, - Name: &isDependency.DependencyPackage.Namespaces[0].Names[0].Name, - } - - depPkgResponse, err := model.Packages(ctx, gqlclient, *depPkgFilter) - if err != nil { - return nil, nil, fmt.Errorf("error querying for dependent package: %w", err) - } + var matchingDepPkgVersionIDs []string + if len(isDependency.DependencyPackage.Namespaces[0].Names[0].Versions) == 0 { + depPkgFilter := &model.PkgSpec{ + Type: &isDependency.DependencyPackage.Type, + Namespace: &isDependency.DependencyPackage.Namespaces[0].Namespace, + Name: &isDependency.DependencyPackage.Namespaces[0].Names[0].Name, + } - depPkgVersionsMap := map[string]string{} - depPkgVersions := []string{} - for _, depPkgVersion := range depPkgResponse.Packages[0].Namespaces[0].Names[0].Versions { - depPkgVersions = append(depPkgVersions, depPkgVersion.Version) - depPkgVersionsMap[depPkgVersion.Version] = depPkgVersion.Id - } + depPkgResponse, err := model.Packages(ctx, gqlclient, *depPkgFilter) + if err != nil { + return nil, nil, fmt.Errorf("error querying for dependent package: %w", err) + } - matchingDepPkgVersions, err := depversion.WhichVersionMatches(depPkgVersions, isDependency.VersionRange) - if err != nil { - // TODO(jeffmendoza): depversion is not handling all/new possible - // version ranges from deps.dev. Continue here to report possible - // vulns even if some paths cannot be followed. - matchingDepPkgVersions = nil - //return nil, nil, fmt.Errorf("error determining dependent version matches: %w", err) - } + depPkgVersionsMap := map[string]string{} + depPkgVersions := []string{} + for _, depPkgVersion := range depPkgResponse.Packages[0].Namespaces[0].Names[0].Versions { + depPkgVersions = append(depPkgVersions, depPkgVersion.Version) + depPkgVersionsMap[depPkgVersion.Version] = depPkgVersion.Id + } - for matchingDepPkgVersion := range matchingDepPkgVersions { - matchingDepPkgVersionID := depPkgVersionsMap[matchingDepPkgVersion] - vulnPath, foundVulnTableRow, err := queryVulnsViaPackageNeighbors(ctx, gqlclient, matchingDepPkgVersionID, []model.Edge{model.EdgePackageCertifyVuln, model.EdgePackageCertifyVexStatement}) + matchingDepPkgVersions, err := depversion.WhichVersionMatches(depPkgVersions, isDependency.VersionRange) if err != nil { - return nil, nil, fmt.Errorf("error querying neighbor: %w", err) - } - if len(vulnPath) > 0 { - path = append(path, isDependency.Id, matchingDepPkgVersionID, - depPkgResponse.Packages[0].Namespaces[0].Names[0].Id, depPkgResponse.Packages[0].Namespaces[0].Id, - depPkgResponse.Packages[0].Id) - path = append(path, vulnPath...) - tableRows = append(tableRows, foundVulnTableRow...) + // TODO(jeffmendoza): depversion is not handling all/new possible + // version ranges from deps.dev. Continue here to report possible + // vulns even if some paths cannot be followed. + matchingDepPkgVersions = nil + //return nil, nil, fmt.Errorf("error determining dependent version matches: %w", err) } + for matchingDepPkgVersion := range matchingDepPkgVersions { + matchingDepPkgVersionIDs = append(matchingDepPkgVersionIDs, depPkgVersionsMap[matchingDepPkgVersion]) + } + } else { + matchingDepPkgVersionIDs = append(matchingDepPkgVersionIDs, isDependency.DependencyPackage.Namespaces[0].Names[0].Versions[0].Id) + } + for _, matchingDepPkgVersionID := range matchingDepPkgVersionIDs { dfsN, seen := nodeMap[matchingDepPkgVersionID] if !seen { dfsN = dfsNode{ @@ -314,14 +310,26 @@ func searchDependencyPackages(ctx context.Context, gqlclient graphql.Client, top depth: nowNode.depth + 1, } nodeMap[matchingDepPkgVersionID] = dfsN + } else { + continue } if !dfsN.expanded { queue = append(queue, matchingDepPkgVersionID) } + vulnPath, foundVulnTableRow, err := queryVulnsViaPackageNeighbors(ctx, gqlclient, matchingDepPkgVersionID, []model.Edge{model.EdgePackageCertifyVuln, model.EdgePackageCertifyVexStatement}) + if err != nil { + return nil, nil, fmt.Errorf("error querying neighbor: %w", err) + } + if len(vulnPath) > 0 { + path = append(path, isDependency.Id, matchingDepPkgVersionID, + isDependency.DependencyPackage.Namespaces[0].Names[0].Id, isDependency.DependencyPackage.Namespaces[0].Id, + isDependency.DependencyPackage.Id) + path = append(path, vulnPath...) + tableRows = append(tableRows, foundVulnTableRow...) + } } } } - nowNode.expanded = true nodeMap[now] = nowNode } @@ -413,38 +421,11 @@ func searchDependencyPackagesReverse(ctx context.Context, gqlclient graphql.Clie break } - pkgNameNeighborResponses, err := model.Neighbors(ctx, gqlclient, now, []model.Edge{}) + isDependencyNeighborResponses, err := model.Neighbors(ctx, gqlclient, now, []model.Edge{model.EdgePackageIsDependency}) if err != nil { return nil, fmt.Errorf("failed getting package parent:%v", err) } - - for _, neighbor := range pkgNameNeighborResponses.Neighbors { - if pkgName, ok := neighbor.(*model.NeighborsNeighborsPackage); ok { - if len(pkgName.Namespaces) == 0 { - continue - } - isDependencyNeighborResponses, err := model.Neighbors(ctx, gqlclient, pkgName.Namespaces[0].Names[0].Id, []model.Edge{model.EdgePackageIsDependency}) - if err != nil { - return nil, fmt.Errorf("failed getting package parent:%v", err) - } - for _, neighbor := range isDependencyNeighborResponses.Neighbors { - if isDependency, ok := neighbor.(*model.NeighborsNeighborsIsDependency); ok && now != isDependency.Package.Namespaces[0].Names[0].Versions[0].Id { - dfsN, seen := nodeMap[isDependency.Package.Namespaces[0].Names[0].Versions[0].Id] - if !seen { - dfsN = dfsNode{ - parent: now, - isDependency: isDependency, - depth: nowNode.depth + 1, - } - nodeMap[isDependency.Package.Namespaces[0].Names[0].Versions[0].Id] = dfsN - } - if !dfsN.expanded { - queue = append(queue, isDependency.Package.Namespaces[0].Names[0].Versions[0].Id) - collectedIDs = append(collectedIDs, isDependency.Package.Namespaces[0].Names[0].Versions[0].Id) - } - } - } - } + for _, neighbor := range isDependencyNeighborResponses.Neighbors { if isDependency, ok := neighbor.(*model.NeighborsNeighborsIsDependency); ok && now != isDependency.Package.Namespaces[0].Names[0].Versions[0].Id { dfsN, seen := nodeMap[isDependency.Package.Namespaces[0].Names[0].Versions[0].Id] if !seen { @@ -460,9 +441,7 @@ func searchDependencyPackagesReverse(ctx context.Context, gqlclient graphql.Clie collectedIDs = append(collectedIDs, isDependency.Package.Namespaces[0].Names[0].Versions[0].Id) } } - } - nowNode.expanded = true nodeMap[now] = nowNode } @@ -475,22 +454,41 @@ func searchDependencyPackagesReverse(ctx context.Context, gqlclient graphql.Clie if topPkgID != "" { now = topPkgID for now != searchPkgID { - path = append(path, nodeMap[now].isDependency.Id, nodeMap[now].isDependency.DependencyPackage.Namespaces[0].Names[0].Id, - nodeMap[now].isDependency.DependencyPackage.Namespaces[0].Id, nodeMap[now].isDependency.DependencyPackage.Id, - nodeMap[now].isDependency.Package.Namespaces[0].Names[0].Versions[0].Id, - nodeMap[now].isDependency.Package.Namespaces[0].Names[0].Id, nodeMap[now].isDependency.Package.Namespaces[0].Id, - nodeMap[now].isDependency.Package.Id) + if len(nodeMap[now].isDependency.DependencyPackage.Namespaces[0].Names[0].Versions) > 0 { + path = append(path, nodeMap[now].isDependency.Id, nodeMap[now].isDependency.DependencyPackage.Namespaces[0].Names[0].Versions[0].Id, + nodeMap[now].isDependency.DependencyPackage.Namespaces[0].Names[0].Id, + nodeMap[now].isDependency.DependencyPackage.Namespaces[0].Id, nodeMap[now].isDependency.DependencyPackage.Id, + nodeMap[now].isDependency.Package.Namespaces[0].Names[0].Versions[0].Id, + nodeMap[now].isDependency.Package.Namespaces[0].Names[0].Id, nodeMap[now].isDependency.Package.Namespaces[0].Id, + nodeMap[now].isDependency.Package.Id) + } else { + path = append(path, nodeMap[now].isDependency.Id, nodeMap[now].isDependency.DependencyPackage.Namespaces[0].Names[0].Id, + nodeMap[now].isDependency.DependencyPackage.Namespaces[0].Id, nodeMap[now].isDependency.DependencyPackage.Id, + nodeMap[now].isDependency.Package.Namespaces[0].Names[0].Versions[0].Id, + nodeMap[now].isDependency.Package.Namespaces[0].Names[0].Id, nodeMap[now].isDependency.Package.Namespaces[0].Id, + nodeMap[now].isDependency.Package.Id) + } + now = nodeMap[now].parent } return path, nil } else { for i := len(collectedIDs) - 1; i >= 0; i-- { if nodeMap[collectedIDs[i]].isDependency != nil { - path = append(path, nodeMap[collectedIDs[i]].isDependency.Id, nodeMap[collectedIDs[i]].isDependency.DependencyPackage.Namespaces[0].Names[0].Id, - nodeMap[collectedIDs[i]].isDependency.DependencyPackage.Namespaces[0].Id, nodeMap[collectedIDs[i]].isDependency.DependencyPackage.Id, - nodeMap[collectedIDs[i]].isDependency.Package.Namespaces[0].Names[0].Versions[0].Id, - nodeMap[collectedIDs[i]].isDependency.Package.Namespaces[0].Names[0].Id, nodeMap[collectedIDs[i]].isDependency.Package.Namespaces[0].Id, - nodeMap[collectedIDs[i]].isDependency.Package.Id) + if len(nodeMap[collectedIDs[i]].isDependency.DependencyPackage.Namespaces[0].Names[0].Versions) > 0 { + path = append(path, nodeMap[collectedIDs[i]].isDependency.Id, nodeMap[collectedIDs[i]].isDependency.DependencyPackage.Namespaces[0].Names[0].Versions[0].Id, + nodeMap[collectedIDs[i]].isDependency.DependencyPackage.Namespaces[0].Names[0].Id, + nodeMap[collectedIDs[i]].isDependency.DependencyPackage.Namespaces[0].Id, nodeMap[collectedIDs[i]].isDependency.DependencyPackage.Id, + nodeMap[collectedIDs[i]].isDependency.Package.Namespaces[0].Names[0].Versions[0].Id, + nodeMap[collectedIDs[i]].isDependency.Package.Namespaces[0].Names[0].Id, nodeMap[collectedIDs[i]].isDependency.Package.Namespaces[0].Id, + nodeMap[collectedIDs[i]].isDependency.Package.Id) + } else { + path = append(path, nodeMap[collectedIDs[i]].isDependency.Id, nodeMap[collectedIDs[i]].isDependency.DependencyPackage.Namespaces[0].Names[0].Id, + nodeMap[collectedIDs[i]].isDependency.DependencyPackage.Namespaces[0].Id, nodeMap[collectedIDs[i]].isDependency.DependencyPackage.Id, + nodeMap[collectedIDs[i]].isDependency.Package.Namespaces[0].Names[0].Versions[0].Id, + nodeMap[collectedIDs[i]].isDependency.Package.Namespaces[0].Names[0].Id, nodeMap[collectedIDs[i]].isDependency.Package.Namespaces[0].Id, + nodeMap[collectedIDs[i]].isDependency.Package.Id) + } } } return path, nil diff --git a/pkg/assembler/helpers/purl_test.go b/pkg/assembler/helpers/purl_test.go index 2ca80024fd..65cd79407a 100644 --- a/pkg/assembler/helpers/purl_test.go +++ b/pkg/assembler/helpers/purl_test.go @@ -34,6 +34,9 @@ func TestPurlConvert(t *testing.T) { expected *model.PkgInputSpec }{ { + purlUri: "pkg:maven/US_export_policy/US_export_policy", + expected: pkg("maven", "US_export_policy", "US_export_policy", "", "", map[string]string{}), + }, { // alpine purlUri: "pkg:alpm/arch/pacman@6.0.1-1?arch=x86_64", expected: pkg("alpm", "arch", "pacman", "6.0.1-1", "", map[string]string{