diff --git a/pkg/analyzer/lfp_npm.go b/pkg/analyzer/lfp_npm.go index 59b68c00..1577dfc2 100644 --- a/pkg/analyzer/lfp_npm.go +++ b/pkg/analyzer/lfp_npm.go @@ -6,6 +6,7 @@ import ( "fmt" "net/url" "os" + "slices" "strings" jsonreportspec "github.com/safedep/vet/gen/jsonreport" @@ -146,7 +147,7 @@ func (npm *npmLockfilePoisoningAnalyzer) Analyze(manifest *models.PackageManifes }) } - if !npmIsUrlFollowsPathConvention(lockfilePackage.Resolved, packageName) { + if !npmIsUrlFollowsPathConvention(lockfilePackage.Resolved, packageName, trustedRegistryUrls) { logger.Debugf("npmLockfilePoisoningAnalyzer: Package [%s] resolved to an unconventional URL [%s]", packageName, lockfilePackage.Resolved) @@ -180,14 +181,7 @@ func (npm *npmLockfilePoisoningAnalyzer) Analyze(manifest *models.PackageManifes // Analyze the artifact URL and determine if the source is trusted func npmIsTrustedSource(sourceUrl string, trusteUrls []string) bool { - // Go url parser cannot handle git+ssh://host:project/repo.git#commit - if len(sourceUrl) > 10 && strings.EqualFold(sourceUrl[0:10], "git+ssh://") { - if cIndex := strings.Index(sourceUrl[10:], ":"); cIndex != -1 { - sourceUrl = sourceUrl[0:10+cIndex] + "/" + sourceUrl[10+cIndex+1:] - } - } - - parsedUrl, err := url.Parse(sourceUrl) + parsedUrl, err := npmParseSourceUrl(sourceUrl) if err != nil { logger.Errorf("npmIsTrustedSource: Failed to parse URL %s: %v", sourceUrl, err) @@ -206,7 +200,7 @@ func npmIsTrustedSource(sourceUrl string, trusteUrls []string) bool { // Compare with trusted URLs for _, trusteUrl := range trusteUrls { - parsedTrustedUrl, err := url.Parse(trusteUrl) + parsedTrustedUrl, err := npmParseSourceUrl(trusteUrl) if err != nil { logger.Errorf("npmIsTrustedSource: Failed to parse trusted URL %s: %v", trusteUrl, err) @@ -235,6 +229,17 @@ func npmIsTrustedSource(sourceUrl string, trusteUrls []string) bool { return false } +func npmParseSourceUrl(sourceUrl string) (*url.URL, error) { + // Go url parser cannot handle git+ssh://host:project/repo.git#commit + if len(sourceUrl) > 10 && strings.EqualFold(sourceUrl[0:10], "git+ssh://") { + if cIndex := strings.Index(sourceUrl[10:], ":"); cIndex != -1 { + sourceUrl = sourceUrl[0:10+cIndex] + "/" + sourceUrl[10+cIndex+1:] + } + } + + return url.Parse(sourceUrl) +} + // Extract the package name from the node_modules filesystem path func npmNodeModulesPackagePathToName(path string) string { return utils.NpmNodeModulesPackagePathToName(path) @@ -242,9 +247,9 @@ func npmNodeModulesPackagePathToName(path string) string { // Test if URL follows the pkg name path convention as per NPM package registry // specification https://docs.npmjs.com/cli/v10/configuring-npm/package-lock-json -func npmIsUrlFollowsPathConvention(sourceUrl string, pkg string) bool { +func npmIsUrlFollowsPathConvention(sourceUrl string, pkg string, trustedUrls []string) bool { // Example: https://registry.npmjs.org/express/-/express-4.17.1.tgz - parsedUrl, err := url.Parse(sourceUrl) + parsedUrl, err := npmParseSourceUrl(sourceUrl) if err != nil { logger.Errorf("npmIsUrlFollowsPathConvention: Failed to parse URL %s: %v", sourceUrl, err) @@ -260,6 +265,25 @@ func npmIsUrlFollowsPathConvention(sourceUrl string, pkg string) bool { path = path[1:] } + acceptablePackageNames := []string{pkg} + for _, trustedUrl := range trustedUrls { + parsedTrustedUrl, err := npmParseSourceUrl(trustedUrl) + if err != nil { + logger.Errorf("npmIsUrlFollowsPathConvention: Failed to parse trusted URL %s: %v", + trustedUrl, err) + continue + } + + trustedBase := parsedTrustedUrl.Path + trustedBase = strings.TrimPrefix(trustedBase, "/") + trustedBase = strings.TrimSuffix(trustedBase, "/") + + acceptablePackageNames = append(acceptablePackageNames, + fmt.Sprintf("%s/%s", trustedBase, pkg)) + } + + // Example: @angular/core from https://registry.npmjs.org/@angular/core/-/core-1.0.0.tgz scopedPackageName := strings.Split(path, "/-/")[0] - return scopedPackageName == pkg + + return slices.Contains(acceptablePackageNames, scopedPackageName) } diff --git a/pkg/analyzer/lfp_npm_test.go b/pkg/analyzer/lfp_npm_test.go index bcb21834..47291452 100644 --- a/pkg/analyzer/lfp_npm_test.go +++ b/pkg/analyzer/lfp_npm_test.go @@ -73,6 +73,12 @@ func TestNpmIsTrustedSource(t *testing.T) { []string{"https://registry.npmjs.org", "git+ssh://github.com/safedep"}, false, }, + { + "source is trusted when trusted url has a base path", + "https://registry.example.org/base/a/b/-/c.tgz", + []string{"https://registry.example.org/base"}, + true, + }, } for _, test := range cases { @@ -85,34 +91,73 @@ func TestNpmIsTrustedSource(t *testing.T) { func TestNpmIsUrlFollowsPathConvention(t *testing.T) { cases := []struct { - name string - url string - pkgName string - expected bool + name string + url string + pkgName string + trustedUrls []string + expected bool }{ { "package name matches url path", "https://registry.npmjs.org/package-name/-/package-name-1.0.0.tgz", "package-name", + []string{}, true, }, { "package name matches scoped url path", "https://registry.npmjs.org/@angular/core/-/core-1.0.0.tgz", "@angular/core", + []string{}, true, }, { "package name does not match scoped url path", "https://registry.npmjs.org/@angular/core/-/core-1.0.0.tgz", "@someother/core", + []string{}, false, }, + { + "package path matches trusted url path", + "https://registry.npmjs.org/base/package-name/-/package-name-1.0.0.tgz", + "package-name", + []string{"https://registry.npmjs.org/base"}, + true, + }, + { + "package path matches trusted url path with trailing slash", + "https://registry.npmjs.org/base/package-name/-/package-name-1.0.0.tgz", + "package-name", + []string{"https://registry.npmjs.org/base/"}, + true, + }, + { + "package path matches trusted url path prefix", + "https://registry.npmjs.org/base/package-name/-/package-name-1.0.0.tgz", + "package-name", + []string{"https://registry.npmjs.org/base1/base2"}, + false, + }, + { + "package path has base without trusted url", + "https://registry.npmjs.org/base/package-name/-/package-name-1.0.0.tgz", + "package-name", + []string{}, + false, + }, + { + "package path matches one of the trusted url base", + "https://registry.npmjs.org/base/package-name/-/package-name-1.0.0.tgz", + "package-name", + []string{"https://registry.npmjs.org/base", "https://registry.npmjs.org/base1"}, + true, + }, } for _, test := range cases { t.Run(test.name, func(t *testing.T) { - actual := npmIsUrlFollowsPathConvention(test.url, test.pkgName) + actual := npmIsUrlFollowsPathConvention(test.url, test.pkgName, test.trustedUrls) assert.Equal(t, test.expected, actual) }) }