From 27db6bf0371d6adba462d8b38399031344cebcfa Mon Sep 17 00:00:00 2001 From: Xueqin Cui <72771658+cuixq@users.noreply.github.com> Date: Wed, 19 Jun 2024 11:28:41 +1000 Subject: [PATCH] Invoke `MavenResolverExtrator` when scanning pom.xml (#1028) https://github.com/google/osv-scanner/issues/35 In this PR, `MavenResolverExtrator` is invoked when scanning pom.xml to report vulnerabilities in transitive dependencies. However, the default Maven extractor is still being used with offline mode. --- cmd/osv-scanner/__snapshots__/main_test.snap | 47 +++++++++++++++++++ .../fixtures/maven-transitive/abc.xml | 23 +++++++++ .../fixtures/maven-transitive/pom.xml | 23 +++++++++ cmd/osv-scanner/main_test.go | 30 ++++++++++++ .../resolution/datasource/maven_registry.go | 8 ++-- pkg/osvscanner/osvscanner.go | 46 ++++++++++++++++-- 6 files changed, 170 insertions(+), 7 deletions(-) create mode 100644 cmd/osv-scanner/fixtures/maven-transitive/abc.xml create mode 100644 cmd/osv-scanner/fixtures/maven-transitive/pom.xml diff --git a/cmd/osv-scanner/__snapshots__/main_test.snap b/cmd/osv-scanner/__snapshots__/main_test.snap index 367dbb0e0f..0f56fe9599 100755 --- a/cmd/osv-scanner/__snapshots__/main_test.snap +++ b/cmd/osv-scanner/__snapshots__/main_test.snap @@ -1513,6 +1513,53 @@ No issues found --- +[TestRun_MavenTransitive/does_not_scan_transitive_dependencies_for_pom.xml_with_offline_mode - 1] +Scanning dir ./fixtures/maven-transitive/pom.xml +Scanned /fixtures/maven-transitive/pom.xml file and found 1 package +Loaded Maven local db from /osv-scanner/Maven/all.zip +No issues found + +--- + +[TestRun_MavenTransitive/does_not_scan_transitive_dependencies_for_pom.xml_with_offline_mode - 2] + +--- + +[TestRun_MavenTransitive/scans_transitive_dependencies_by_specifying_pom.xml - 1] +Scanned /fixtures/maven-transitive/abc.xml file as a pom.xml and found 3 packages ++-------------------------------------+------+-----------+-------------------------------------+---------+-----------------------------------+ +| OSV URL | CVSS | ECOSYSTEM | PACKAGE | VERSION | SOURCE | ++-------------------------------------+------+-----------+-------------------------------------+---------+-----------------------------------+ +| https://osv.dev/GHSA-7rjr-3q55-vv33 | 9.0 | Maven | org.apache.logging.log4j:log4j-core | 2.14.1 | fixtures/maven-transitive/abc.xml | +| https://osv.dev/GHSA-8489-44mv-ggj8 | 6.6 | Maven | org.apache.logging.log4j:log4j-core | 2.14.1 | fixtures/maven-transitive/abc.xml | +| https://osv.dev/GHSA-jfh8-c2jp-5v3q | 10.0 | Maven | org.apache.logging.log4j:log4j-core | 2.14.1 | fixtures/maven-transitive/abc.xml | +| https://osv.dev/GHSA-p6xc-xr62-6r2g | 8.6 | Maven | org.apache.logging.log4j:log4j-core | 2.14.1 | fixtures/maven-transitive/abc.xml | ++-------------------------------------+------+-----------+-------------------------------------+---------+-----------------------------------+ + +--- + +[TestRun_MavenTransitive/scans_transitive_dependencies_by_specifying_pom.xml - 2] + +--- + +[TestRun_MavenTransitive/scans_transitive_dependencies_for_pom.xml_by_default - 1] +Scanning dir ./fixtures/maven-transitive/pom.xml +Scanned /fixtures/maven-transitive/pom.xml file and found 3 packages ++-------------------------------------+------+-----------+-------------------------------------+---------+-----------------------------------+ +| OSV URL | CVSS | ECOSYSTEM | PACKAGE | VERSION | SOURCE | ++-------------------------------------+------+-----------+-------------------------------------+---------+-----------------------------------+ +| https://osv.dev/GHSA-7rjr-3q55-vv33 | 9.0 | Maven | org.apache.logging.log4j:log4j-core | 2.14.1 | fixtures/maven-transitive/pom.xml | +| https://osv.dev/GHSA-8489-44mv-ggj8 | 6.6 | Maven | org.apache.logging.log4j:log4j-core | 2.14.1 | fixtures/maven-transitive/pom.xml | +| https://osv.dev/GHSA-jfh8-c2jp-5v3q | 10.0 | Maven | org.apache.logging.log4j:log4j-core | 2.14.1 | fixtures/maven-transitive/pom.xml | +| https://osv.dev/GHSA-p6xc-xr62-6r2g | 8.6 | Maven | org.apache.logging.log4j:log4j-core | 2.14.1 | fixtures/maven-transitive/pom.xml | ++-------------------------------------+------+-----------+-------------------------------------+---------+-----------------------------------+ + +--- + +[TestRun_MavenTransitive/scans_transitive_dependencies_for_pom.xml_by_default - 2] + +--- + [TestRun_OCIImage/Alpine_3.10_image_tar_with_3.18_version_file - 1] Scanning image ../../internal/image/fixtures/test-alpine.tar +--------------------------------+------+--------------+---------+-----------+---------------------------------------------------------------------+ diff --git a/cmd/osv-scanner/fixtures/maven-transitive/abc.xml b/cmd/osv-scanner/fixtures/maven-transitive/abc.xml new file mode 100644 index 0000000000..ad00ad3329 --- /dev/null +++ b/cmd/osv-scanner/fixtures/maven-transitive/abc.xml @@ -0,0 +1,23 @@ + + + + 4.0.0 + + com.mycompany.app + my-app + 1.0.0 + + my-app + + http://www.example.com + + + + org.apache.logging.log4j + log4j-web + 2.14.1 + + + + diff --git a/cmd/osv-scanner/fixtures/maven-transitive/pom.xml b/cmd/osv-scanner/fixtures/maven-transitive/pom.xml new file mode 100644 index 0000000000..ad00ad3329 --- /dev/null +++ b/cmd/osv-scanner/fixtures/maven-transitive/pom.xml @@ -0,0 +1,23 @@ + + + + 4.0.0 + + com.mycompany.app + my-app + 1.0.0 + + my-app + + http://www.example.com + + + + org.apache.logging.log4j + log4j-web + 2.14.1 + + + + diff --git a/cmd/osv-scanner/main_test.go b/cmd/osv-scanner/main_test.go index 816bf7377c..405b289b5f 100644 --- a/cmd/osv-scanner/main_test.go +++ b/cmd/osv-scanner/main_test.go @@ -796,3 +796,33 @@ func TestRun_InsertDefaultCommand(t *testing.T) { testutility.NewSnapshot().MatchText(t, normalizeStdStream(t, stderr)) } } + +func TestRun_MavenTransitive(t *testing.T) { + t.Parallel() + tests := []cliTestCase{ + { + name: "scans transitive dependencies for pom.xml by default", + args: []string{"", "./fixtures/maven-transitive/pom.xml"}, + exit: 1, + }, + { + name: "scans transitive dependencies by specifying pom.xml", + args: []string{"", "-L", "pom.xml:./fixtures/maven-transitive/abc.xml"}, + exit: 1, + }, + { + // Direct dependencies do not have any vulnerability. + name: "does not scan transitive dependencies for pom.xml with offline mode", + args: []string{"", "--experimental-offline", "--experimental-download-offline-databases", "./fixtures/maven-transitive/pom.xml"}, + exit: 0, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + testCli(t, tt) + }) + } +} diff --git a/internal/resolution/datasource/maven_registry.go b/internal/resolution/datasource/maven_registry.go index 9990bd3b1a..bcc6309b48 100644 --- a/internal/resolution/datasource/maven_registry.go +++ b/internal/resolution/datasource/maven_registry.go @@ -3,13 +3,13 @@ package datasource import ( "context" "encoding/xml" + "errors" "fmt" "net/http" "net/url" "strings" "deps.dev/util/maven" - "github.com/google/osv-scanner/pkg/osvscanner" ) const MavenCentral = "https://repo.maven.apache.org/maven2" @@ -22,6 +22,8 @@ func NewMavenRegistryAPIClient(registry string) *MavenRegistryAPIClient { return &MavenRegistryAPIClient{registry: registry} } +var errAPIFailed = errors.New("API query failed") + func (m *MavenRegistryAPIClient) GetProject(ctx context.Context, groupID, artifactID, version string) (maven.Project, error) { u, err := url.JoinPath(m.registry, strings.ReplaceAll(groupID, ".", "/"), artifactID, version, fmt.Sprintf("%s-%s.pom", artifactID, version)) if err != nil { @@ -35,12 +37,12 @@ func (m *MavenRegistryAPIClient) GetProject(ctx context.Context, groupID, artifa resp, err := http.DefaultClient.Do(req) if err != nil { - return maven.Project{}, fmt.Errorf("%w: Maven registry query failed: %w", osvscanner.ErrAPIFailed, err) + return maven.Project{}, fmt.Errorf("%w: Maven registry query failed: %w", errAPIFailed, err) } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return maven.Project{}, fmt.Errorf("%w: Maven registry query status: %s", osvscanner.ErrAPIFailed, resp.Status) + return maven.Project{}, fmt.Errorf("%w: Maven registry query status: %s", errAPIFailed, resp.Status) } var proj maven.Project diff --git a/pkg/osvscanner/osvscanner.go b/pkg/osvscanner/osvscanner.go index 86584985a4..04800bae60 100644 --- a/pkg/osvscanner/osvscanner.go +++ b/pkg/osvscanner/osvscanner.go @@ -10,12 +10,16 @@ import ( "os/exec" "path" "path/filepath" + "sort" "strings" "github.com/google/osv-scanner/internal/customgitignore" "github.com/google/osv-scanner/internal/image" "github.com/google/osv-scanner/internal/local" + "github.com/google/osv-scanner/internal/manifest" "github.com/google/osv-scanner/internal/output" + "github.com/google/osv-scanner/internal/resolution/client" + "github.com/google/osv-scanner/internal/resolution/datasource" "github.com/google/osv-scanner/internal/sbom" "github.com/google/osv-scanner/internal/semantic" "github.com/google/osv-scanner/internal/version" @@ -160,7 +164,7 @@ func scanDir(r reporter.Reporter, dir string, skipGit bool, recursive bool, useG if !info.IsDir() { if extractor, _ := lockfile.FindExtractor(path, ""); extractor != nil { - pkgs, err := scanLockfile(r, path, "") + pkgs, err := scanLockfile(r, path, "", compareOffline) if err != nil { r.Errorf("Attempted to scan lockfile but failed: %s\n", path) } @@ -341,7 +345,7 @@ func scanImage(r reporter.Reporter, path string) ([]scannedPackage, error) { // scanLockfile will load, identify, and parse the lockfile path passed in, and add the dependencies specified // within to `query` -func scanLockfile(r reporter.Reporter, path string, parseAs string) ([]scannedPackage, error) { +func scanLockfile(r reporter.Reporter, path string, parseAs string, compareOffline bool) ([]scannedPackage, error) { var err error var parsedLockfile lockfile.Lockfile @@ -359,7 +363,11 @@ func scanLockfile(r reporter.Reporter, path string, parseAs string) ([]scannedPa case "osv-scanner": parsedLockfile, err = lockfile.FromOSVScannerResults(path) default: - parsedLockfile, err = lockfile.ExtractDeps(f, parseAs) + if !compareOffline && (parseAs == "pom.xml" || filepath.Base(path) == "pom.xml") { + parsedLockfile, err = extractMavenDeps(f) + } else { + parsedLockfile, err = lockfile.ExtractDeps(f, parseAs) + } } } @@ -399,6 +407,36 @@ func scanLockfile(r reporter.Reporter, path string, parseAs string) ([]scannedPa return packages, nil } +func extractMavenDeps(f lockfile.DepFile) (lockfile.Lockfile, error) { + depClient, err := client.NewDepsDevClient(depsdev.DepsdevAPI) + if err != nil { + return lockfile.Lockfile{}, err + } + extractor := manifest.MavenResolverExtractor{ + DependencyClient: depClient, + MavenRegistryAPIClient: *datasource.NewMavenRegistryAPIClient(datasource.MavenCentral), + } + packages, err := extractor.Extract(f) + if err != nil { + err = fmt.Errorf("failed extracting %s: %w", f.Path(), err) + } + + // Sort packages for testing convenience. + sort.Slice(packages, func(i, j int) bool { + if packages[i].Name == packages[j].Name { + return packages[i].Version < packages[j].Version + } + + return packages[i].Name < packages[j].Name + }) + + return lockfile.Lockfile{ + FilePath: f.Path(), + ParsedAs: "pom.xml", + Packages: packages, + }, err +} + // scanSBOMFile will load, identify, and parse the SBOM path passed in, and add the dependencies specified // within to `query` func scanSBOMFile(r reporter.Reporter, path string, fromFSScan bool) ([]scannedPackage, error) { @@ -803,7 +841,7 @@ func DoScan(actions ScannerActions, r reporter.Reporter) (models.VulnerabilityRe r.Errorf("Failed to resolved path with error %s\n", err) return models.VulnerabilityResults{}, err } - pkgs, err := scanLockfile(r, lockfilePath, parseAs) + pkgs, err := scanLockfile(r, lockfilePath, parseAs, actions.CompareOffline) if err != nil { return models.VulnerabilityResults{}, err }