diff --git a/pkg/analyzer/analyzer.go b/pkg/analyzer/analyzer.go index 482260b..23a1267 100644 --- a/pkg/analyzer/analyzer.go +++ b/pkg/analyzer/analyzer.go @@ -9,6 +9,7 @@ type AnalyzerEventType string const ( ET_FilterExpressionMatched = AnalyzerEventType("ev_pkg_filter_match") + ET_LockfilePoisoningSignal = AnalyzerEventType("ev_lockfile_poisoning") ET_AnalyzerFailOnError = AnalyzerEventType("ev_fail_on_error") ) diff --git a/pkg/analyzer/event.go b/pkg/analyzer/event.go index f829398..081736c 100644 --- a/pkg/analyzer/event.go +++ b/pkg/analyzer/event.go @@ -7,3 +7,7 @@ func (ev *AnalyzerEvent) IsFailOnError() bool { func (ev *AnalyzerEvent) IsFilterMatch() bool { return ev.Type == ET_FilterExpressionMatched } + +func (ev *AnalyzerEvent) IsLockfilePoisoningSignal() bool { + return ev.Type == ET_LockfilePoisoningSignal +} diff --git a/pkg/analyzer/lfp.go b/pkg/analyzer/lfp.go new file mode 100644 index 0000000..d64dfb3 --- /dev/null +++ b/pkg/analyzer/lfp.go @@ -0,0 +1,60 @@ +package analyzer + +import ( + specmodels "github.com/safedep/vet/gen/models" + "github.com/safedep/vet/pkg/common/logger" + "github.com/safedep/vet/pkg/models" +) + +const lfpAnalyzerName = "LockfilePoisoningAnalyzer" + +type LockfilePoisoningAnalyzerConfig struct { + FailFast bool +} + +type lockfilePoisoningAnalyzer struct { + config LockfilePoisoningAnalyzerConfig +} + +type lockfilePoisoningAnalyzerPlugin interface { + Analyze(manifest *models.PackageManifest, handler AnalyzerEventHandler) error +} + +type lockfileAnalyzerPluginBuilder = func(config *LockfilePoisoningAnalyzerConfig) lockfilePoisoningAnalyzerPlugin + +var lockfilePoisoningAnalyzers = map[specmodels.Ecosystem]lockfileAnalyzerPluginBuilder{ + specmodels.Ecosystem_Npm: func(config *LockfilePoisoningAnalyzerConfig) lockfilePoisoningAnalyzerPlugin { + return &npmLockfilePoisoningAnalyzer{ + config: *config, + } + }, +} + +func NewLockfilePoisoningAnalyzer(config LockfilePoisoningAnalyzerConfig) (Analyzer, error) { + return &lockfilePoisoningAnalyzer{ + config: config, + }, nil +} + +func (lfp *lockfilePoisoningAnalyzer) Name() string { + return lfpAnalyzerName +} + +func (lfp *lockfilePoisoningAnalyzer) Analyze(manifest *models.PackageManifest, + handler AnalyzerEventHandler) error { + logger.Debugf("LockfilePoisoningAnalyzer: Analyzing [%s] %s", + manifest.GetSpecEcosystem(), manifest.GetDisplayPath()) + + pluginBuilder, ok := lockfilePoisoningAnalyzers[manifest.GetSpecEcosystem()] + if !ok { + logger.Warnf("No lockfile poisoning analyzer for ecosystem %s", manifest.Ecosystem) + return nil + } + + plugin := pluginBuilder(&lfp.config) + return plugin.Analyze(manifest, handler) +} + +func (lfp *lockfilePoisoningAnalyzer) Finish() error { + return nil +} diff --git a/pkg/analyzer/lfp_npm.go b/pkg/analyzer/lfp_npm.go new file mode 100644 index 0000000..4eea6d5 --- /dev/null +++ b/pkg/analyzer/lfp_npm.go @@ -0,0 +1,234 @@ +package analyzer + +import ( + "bytes" + "encoding/json" + "fmt" + "net/url" + "os" + "strings" + + "github.com/safedep/vet/pkg/common/logger" + "github.com/safedep/vet/pkg/models" + "github.com/safedep/vet/pkg/readers" +) + +const npmRegistryTrustedUrlBase = "https://registry.npmjs.org" + +type npmPackageLockPackage struct { + Version string `json:"version"` + License string `json:"license"` + Resolved string `json:"resolved"` + Integrity string `json:"integrity"` + Dev bool `json:"dev"` + Optional bool `json:"optional"` +} + +// https://docs.npmjs.com/cli/v10/configuring-npm/package-lock-json +type npmPackageLock struct { + Name string `json:"name"` + Version string `json:"version"` + LockfileVersion int `json:"lockfileVersion"` + Packages map[string]npmPackageLockPackage `json:"packages"` +} + +type npmLockfilePoisoningAnalyzer struct { + config LockfilePoisoningAnalyzerConfig +} + +func (npm *npmLockfilePoisoningAnalyzer) Analyze(manifest *models.PackageManifest, + handler AnalyzerEventHandler) error { + logger.Debugf("npmLockfilePoisoningAnalyzer: Analyzing [%s] %s", + manifest.GetSpecEcosystem(), manifest.GetDisplayPath()) + + data, err := os.ReadFile(manifest.GetPath()) + if err != nil { + return err + } + + var lockfile npmPackageLock + err = json.NewDecoder(bytes.NewReader(data)).Decode(&lockfile) + if err != nil { + return err + } + + if lockfile.LockfileVersion < 2 { + return fmt.Errorf("npmLockfilePoisoningAnalyzer: Unsupported lockfile version %d", + lockfile.LockfileVersion) + } + + logger.Debugf("npmLockfilePoisoningAnalyzer: Found %d packages in lockfile", + len(lockfile.Packages)) + + // Build a map of packages to query by name + pkgMap := map[string]*models.Package{} + err = readers.NewManifestModelReader(manifest).EnumPackages(func(p *models.Package) error { + pkgMap[p.Name] = p + return nil + }) + + if err != nil { + return err + } + + logger.Debugf("npmLockfilePoisoningAnalyzer: Found %d packages in manifest", len(pkgMap)) + + // Poisoning can happen in the following cases: + // 1. If the package is fetched from an untrusted host + // 2. If the package path on filesystem does not match the URL path convention + // 3. If a new entry is added to package-lock.json dependency list + for path, lockfilePackage := range lockfile.Packages { + // The root package is not a dependency, it is the application itself + if path == "" { + logger.Debugf("npmLockfilePoisoningAnalyzer: Skipping root package") + continue + } + + if lockfilePackage.Resolved == "" { + logger.Warnf("npmLockfilePoisoningAnalyzer: Node Module [%s] does not have a resolved URL", path) + continue + } + + packageName := npmNodeModulesPackagePathToName(path) + if packageName == "" { + logger.Warnf("npmLockfilePoisoningAnalyzer: Failed to extract package name from path %s", path) + continue + } + + pkg, ok := pkgMap[packageName] + if !ok { + logger.Warnf("npmLockfilePoisoningAnalyzer: Package [%s] not found in manifest", packageName) + continue + } + + logger.Debugf("npmLockfilePoisoningAnalyzer: Analyzing package [%s]", packageName) + + if !npmIsTrustedSource(lockfilePackage.Resolved, []string{npmRegistryTrustedUrlBase}) { + logger.Debugf("npmLockfilePoisoningAnalyzer: Package [%s] resolved to an untrusted host [%s]", + packageName, lockfilePackage.Resolved) + + _ = handler(&AnalyzerEvent{ + Source: lfpAnalyzerName, + Type: ET_LockfilePoisoningSignal, + Message: fmt.Sprintf("Package [%s] resolved to an untrusted host [%s]", + packageName, lockfilePackage.Resolved), + Manifest: manifest, + Package: pkg, + }) + } + + if !npmIsUrlFollowsPathConvention(lockfilePackage.Resolved, packageName) { + logger.Debugf("npmLockfilePoisoningAnalyzer: Package [%s] resolved to an unconventional URL [%s]", + packageName, lockfilePackage.Resolved) + + _ = handler(&AnalyzerEvent{ + Source: lfpAnalyzerName, + Type: ET_LockfilePoisoningSignal, + Message: fmt.Sprintf("Package [%s] resolved to an URL [%s] that does not follow the "+ + "package name path convention", packageName, lockfilePackage.Resolved), + Manifest: manifest, + Package: pkg, + }) + } + + // TODO: Handle the 3rd case of new entry added to package-lock.json dependency list + } + + return nil +} + +// Analyze the artifact URL and determine if the source is trusted +func npmIsTrustedSource(sourceUrl string, trusteUrls []string) bool { + scheme := "" + host := "" + port := "" + path := "" + + parsedUrl, err := url.Parse(sourceUrl) + if err != nil { + logger.Errorf("npmIsTrustedSource: Failed to parse URL %s: %v", + sourceUrl, err) + return false + } + + scheme = parsedUrl.Scheme + host = parsedUrl.Hostname() + port = parsedUrl.Port() + path = parsedUrl.Path + + // Always true for local filesystem URLs + if scheme == "file" || scheme == "" { + return true + } + + // Compare with trusted URLs + for _, trusteUrl := range trusteUrls { + parsedTrustedUrl, err := url.Parse(trusteUrl) + if err != nil { + logger.Errorf("npmIsTrustedSource: Failed to parse trusted URL %s: %v", + trusteUrl, err) + continue + } + + if parsedTrustedUrl.Scheme != scheme { + continue + } + + if !strings.EqualFold(parsedTrustedUrl.Host, host) { + continue + } + + if parsedTrustedUrl.Port() != "" && parsedTrustedUrl.Port() != port { + continue + } + + if parsedTrustedUrl.Path != "" && !strings.HasPrefix(path, parsedTrustedUrl.Path) { + continue + } + + return true + } + + return false +} + +// Extract the package name from the node_modules filesystem path +func npmNodeModulesPackagePathToName(path string) string { + // Extract the package name from the node_modules filesystem path + // Example: node_modules/express -> express + // Example: node_modules/@angular/core -> @angular/core + // Example: node_modules/@angular/core/node_modules/express -> express + // Example: node_modules/@angular/core/node_modules/@angular/common -> @angular/common + + for i := len(path) - 1; i >= 0; i-- { + if (len(path[i:]) > 13) && (path[i:i+13] == "node_modules/") { + return path[i+13:] + } + } + + return "" +} + +// 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 { + // Example: https://registry.npmjs.org/express/-/express-4.17.1.tgz + parsedUrl, err := url.Parse(sourceUrl) + if err != nil { + logger.Errorf("npmIsUrlFollowsPathConvention: Failed to parse URL %s: %v", + sourceUrl, err) + return false + } + + path := parsedUrl.Path + if path == "" { + return false + } + + if path[0] == '/' { + path = path[1:] + } + + scopedPackageName := strings.Split(path, "/-/")[0] + return scopedPackageName == pkg +} diff --git a/pkg/analyzer/lfp_npm_test.go b/pkg/analyzer/lfp_npm_test.go new file mode 100644 index 0000000..3238cfa --- /dev/null +++ b/pkg/analyzer/lfp_npm_test.go @@ -0,0 +1,144 @@ +package analyzer + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestNpmIsTrustedSource(t *testing.T) { + cases := []struct { + name string + host string + trusted []string + expected bool + }{ + { + "source is trusted if host match is found", + "https://registry.npmjs.org", + []string{"https://registry.npmjs.org"}, + true, + }, + { + "source is not trusted if host does not match", + "https://registry.example.org", + []string{"https://registry.npmjs.com"}, + false, + }, + { + "source is a trusted git url matching path prefix", + "git://github.com/safedep/vet.git", + []string{"git://github.com/safedep"}, + true, + }, + { + "source is a trusted git url but does not match path prefix", + "git://github.com/anything/vet.git", + []string{"github.com/safedep"}, + false, + }, + { + "local urls are always trusted", + "file:///a/b/c", + []string{"registry.npmjs.com"}, + true, + }, + /* + { + "source is a git url with user and commit-ish", + "git+ssh://user@github.com:safedep/project.git#commit-ish", + []string{"github.com/safedep"}, + true, + }, + */ + { + "source is a local relative url", + "./a/b/c", + []string{"https://registry.npmjs.com"}, + true, + }, + } + + for _, test := range cases { + t.Run(test.name, func(t *testing.T) { + actual := npmIsTrustedSource(test.host, test.trusted) + assert.Equal(t, test.expected, actual) + }) + } +} + +func TestNpmNodeModulesPackagePathToName(t *testing.T) { + cases := []struct { + name string + path string + expected string + }{ + { + "package name is extracted from path", + "/a/b/c/node_modules/package-name", + "package-name", + }, + { + "node_modules relative", + "node_modules/express", + "express", + }, + { + "node_modules relative scoped name", + "node_modules/@angular/core", + "@angular/core", + }, + { + "nested node_modules relative", + "node_modules/@angular/core/node_modules/express", + "express", + }, + { + "nested node_modules relative scoped name", + "node_modules/@angular/core/node_modules/@angular/common", + "@angular/common", + }, + } + + for _, test := range cases { + t.Run(test.name, func(t *testing.T) { + actual := npmNodeModulesPackagePathToName(test.path) + assert.Equal(t, test.expected, actual) + }) + } +} + +func TestNpmIsUrlFollowsPathConvention(t *testing.T) { + cases := []struct { + name string + url string + pkgName string + expected bool + }{ + { + "package name matches url path", + "https://registry.npmjs.org/package-name/-/package-name-1.0.0.tgz", + "package-name", + true, + }, + { + "package name matches scoped url path", + "https://registry.npmjs.org/@angular/core/-/core-1.0.0.tgz", + "@angular/core", + true, + }, + { + "package name does not match scoped url path", + "https://registry.npmjs.org/@angular/core/-/core-1.0.0.tgz", + "@someother/core", + false, + }, + } + + for _, test := range cases { + t.Run(test.name, func(t *testing.T) { + actual := npmIsUrlFollowsPathConvention(test.url, test.pkgName) + assert.Equal(t, test.expected, actual) + }) + } +} diff --git a/pkg/reporter/summary.go b/pkg/reporter/summary.go index cea72f1..f3b732e 100644 --- a/pkg/reporter/summary.go +++ b/pkg/reporter/summary.go @@ -74,6 +74,7 @@ type summaryReporter struct { // Map of pkgId and associated meta for building remediation advice remediationScores map[string]*summaryReporterRemediationData violations map[string]*summaryReporterInputViolationData + lockfilePoisoning []string } func NewSummaryReporter(config SummaryReporterConfig) (Reporter, error) { @@ -107,6 +108,10 @@ func (r *summaryReporter) AddManifest(manifest *models.PackageManifest) { } func (r *summaryReporter) AddAnalyzerEvent(event *analyzer.AnalyzerEvent) { + if event.IsLockfilePoisoningSignal() { + r.lockfilePoisoning = append(r.lockfilePoisoning, event.Message.(string)) + } + if !event.IsFilterMatch() { return } @@ -270,6 +275,17 @@ func (r *summaryReporter) Finish() error { fmt.Println() } + if len(r.lockfilePoisoning) > 0 { + fmt.Println(summaryListPrependText, text.Bold.Sprint(" Lockfile Poisoning Detected ")) + fmt.Println() + + for _, msg := range r.lockfilePoisoning { + fmt.Println(text.WrapHard(text.BgRed.Sprint(summaryListPrependText, msg), 120)) + } + + fmt.Println() + } + fmt.Println("Run with `vet --filter=\"...\"` for custom filters to identify risky libraries") fmt.Println("For more details", text.Bold.Sprint("https://github.com/safedep/vet")) fmt.Println() diff --git a/scan.go b/scan.go index 76fdd6a..1b36b90 100644 --- a/scan.go +++ b/scan.go @@ -205,7 +205,16 @@ func internalStartScan() error { readerList = append(readerList, reader) - analyzers := []analyzer.Analyzer{} + // We will always use this analyzer + lfpAnalyzer, err := analyzer.NewLockfilePoisoningAnalyzer(analyzer.LockfilePoisoningAnalyzerConfig{ + FailFast: false, + }) + + if err != nil { + return err + } + + analyzers := []analyzer.Analyzer{lfpAnalyzer} if !utils.IsEmptyString(dumpJsonManifestDir) { task, err := analyzer.NewJsonDumperAnalyzer(dumpJsonManifestDir) if err != nil {