fix: use PEP440 for Python package version comparison (#1510)

Previously, grype used fuzzy matcher for Python packages, since
there are cases in PEP440 that are not strictly semver. Switch to a
library that does PEP440 parsing and comparison for python version 
constraints.

Signed-off-by: Will Murphy <will.murphy@anchore.com>

.gitignore

@@ -4,7 +4,7 @@
 /.grype.yaml
 
 CHANGELOG.md
-VERSION
+/VERSION
 /snapshot/
 /dist/
 *.profile

go.mod

@@ -15,6 +15,7 @@ require (
 	github.com/anchore/packageurl-go v0.1.1-0.20230104203445-02e0a6721501
 	github.com/anchore/stereoscope v0.0.0-20230919183137-5841b53a0375
 	github.com/anchore/syft v0.91.0
+	github.com/aquasecurity/go-pep440-version v0.0.0-20210121094942-22b2f8951d46
 	github.com/bmatcuk/doublestar/v2 v2.0.4
 	github.com/charmbracelet/bubbletea v0.24.2
 	github.com/charmbracelet/lipgloss v0.8.0
@@ -85,7 +86,6 @@ require (
 	github.com/anchore/go-macholibre v0.0.0-20220308212642-53e6d0aaf6fb // indirect
 	github.com/anchore/go-struct-converter v0.0.0-20221118182256-c68fdcfa2092 // indirect
 	github.com/andybalholm/brotli v1.0.4 // indirect
-	github.com/aquasecurity/go-pep440-version v0.0.0-20210121094942-22b2f8951d46 // indirect
 	github.com/aquasecurity/go-version v0.0.0-20210121072130-637058cfe492 // indirect
 	github.com/aws/aws-sdk-go v1.44.288 // indirect
 	github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect

grype/search/cpe_test.go

@@ -102,7 +102,7 @@ func (pr *mockVulnStore) stub() {
 			{
 				PackageName:       "funfun",
 				VersionConstraint: "= 5.2.1",
-				VersionFormat:     version.PythonFormat.String(),
+				VersionFormat:     version.UnknownFormat.String(),
 				ID:                "CVE-2017-fake-6",
 				CPEs: []string{
 					"cpe:2.3:*:funfun:funfun:5.2.1:*:*:*:*:python:*:*",
@@ -574,7 +574,7 @@ func TestFindMatchesByPackageCPE(t *testing.T) {
 									"cpe:2.3:*:funfun:funfun:*:*:*:*:*:python:*:*",
 									"cpe:2.3:*:funfun:funfun:5.2.1:*:*:*:*:python:*:*",
 								},
-								VersionConstraint: "= 5.2.1 (python)",
+								VersionConstraint: "= 5.2.1 (unknown)",
 								VulnerabilityID:   "CVE-2017-fake-6",
 							},
 							Matcher: matcher,

grype/version/constraint.go

@@ -25,7 +25,7 @@ func GetConstraint(constStr string, format Format) (Constraint, error) {
 		// Although this will work in most cases, some oddities aren't supported, like:
 		// 1.0b2.post345.dev456 which is allowed by the spec. In that case (a dev release of a post release)
 		// the comparator will fail. See https://www.python.org/dev/peps/pep-0440
-		return newFuzzyConstraint(constStr, "python")
+		return newPep440Constraint(constStr)
 	case KBFormat:
 		return newKBConstraint(constStr)
 	case PortageFormat:

grype/version/pep440_constraint.go

@@ -0,0 +1,62 @@
+package version
+
+import "fmt"
+
+type pep440Constraint struct {
+	raw        string
+	expression constraintExpression
+}
+
+func (p pep440Constraint) String() string {
+	if p.raw == "" {
+		return "none (python)"
+	}
+	return fmt.Sprintf("%s (python)", p.raw)
+}
+
+func (p pep440Constraint) Satisfied(version *Version) (bool, error) {
+	if p.raw == "" && version != nil {
+		// an empty constraint is always satisfied
+		return true, nil
+	} else if version == nil {
+		if p.raw != "" {
+			// a non-empty constraint with no version given should always fail
+			return false, nil
+		}
+		return true, nil
+	}
+	if version.Format != PythonFormat {
+		return false, fmt.Errorf("(python) unsupported format: %s", version.Format)
+	}
+
+	if version.rich.pep440version == nil {
+		return false, fmt.Errorf("no rich PEP440 version given: %+v", version)
+	}
+	return p.expression.satisfied(version)
+}
+
+var _ Constraint = (*pep440Constraint)(nil)
+
+func newPep440Constraint(raw string) (pep440Constraint, error) {
+	if raw == "" {
+		return pep440Constraint{}, nil
+	}
+
+	constraints, err := newConstraintExpression(raw, newPep440Comparator)
+	if err != nil {
+		return pep440Constraint{}, fmt.Errorf("unable to parse pep440 constrain phrase %w", err)
+	}
+
+	return pep440Constraint{
+		expression: constraints,
+		raw:        raw,
+	}, nil
+}
+
+func newPep440Comparator(unit constraintUnit) (Comparator, error) {
+	ver, err := newPep440Version(unit.version)
+	if err != nil {
+		return nil, fmt.Errorf("unable to parse constraint version (%s): %w", unit.version, err)
+	}
+	return ver, nil
+}

grype/version/pep440_constraint_test.go

@@ -0,0 +1,221 @@
+package version
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestItWorks(t *testing.T) {
+	tests := []testCase{
+		{
+			name:       "empty constraint",
+			version:    "2.3.1",
+			constraint: "",
+			satisfied:  true,
+		},
+		{
+			name:       "version range within",
+			constraint: ">1.0, <2.0",
+			version:    "1.2+beta-3",
+			satisfied:  true,
+		},
+		{
+			name:       "version within compound range",
+			constraint: ">1.0, <2.0 || > 3.0",
+			version:    "3.2+beta-3",
+			satisfied:  true,
+		},
+		{
+			name:       "version within compound range (2)",
+			constraint: ">1.0, <2.0 || > 3.0",
+			version:    "1.2+beta-3",
+			satisfied:  true,
+		},
+		{
+			name:       "version not within compound range",
+			constraint: ">1.0, <2.0 || > 3.0",
+			version:    "2.2+beta-3",
+			satisfied:  false,
+		},
+		{
+			name:       "version range outside (right)",
+			constraint: ">1.0, <2.0",
+			version:    "2.1-beta-3",
+			satisfied:  false,
+		},
+		{
+			name:       "version range outside (left)",
+			constraint: ">1.0, <2.0",
+			version:    "0.9-beta-2",
+			satisfied:  false,
+		},
+		{
+			name:       "version range within (excluding left, prerelease)",
+			constraint: ">=1.0, <2.0",
+			version:    "1.0-beta-3",
+			satisfied:  false,
+		},
+		{
+			name:       "version range within (including left)",
+			constraint: ">=1.1, <2.0",
+			version:    "1.1",
+			satisfied:  true,
+		},
+		{
+			name:       "version range within (excluding right, 1)",
+			constraint: ">1.0, <=2.0",
+			version:    "2.0-beta-3",
+			satisfied:  true,
+		},
+		{
+			name:       "version range within (excluding right, 2)",
+			constraint: ">1.0, <2.0",
+			version:    "2.0-beta-3",
+			satisfied:  true,
+		},
+		{
+			name:       "version range within (including right)",
+			constraint: ">1.0, <=2.0",
+			version:    "2.0",
+			satisfied:  true,
+		},
+		{
+			name:       "version range within (including right, longer version [valid semver, bad fuzzy])",
+			constraint: ">1.0, <=2.0",
+			version:    "2.0.0",
+			satisfied:  true,
+		},
+		{
+			name:       "bad semver (eq)",
+			version:    "5a2",
+			constraint: "=5a2",
+			satisfied:  true,
+		},
+		{
+			name:       "bad semver (gt)",
+			version:    "5a2",
+			constraint: ">5a1",
+			satisfied:  true,
+		},
+		{
+			name:       "bad semver (lt)",
+			version:    "5a2",
+			constraint: "<6a1",
+			satisfied:  true,
+		},
+		{
+			name:       "bad semver (lte)",
+			version:    "5a2",
+			constraint: "<=5a2",
+			satisfied:  true,
+		},
+		{
+			name:       "bad semver (gte)",
+			version:    "5a2",
+			constraint: ">=5a2",
+			satisfied:  true,
+		},
+		{
+			name:       "bad semver (lt boundary)",
+			version:    "5a2",
+			constraint: "<5a2",
+			satisfied:  false,
+		},
+		// regression for https://github.com/anchore/go-version/pull/2
+		{
+			name:       "indirect package match",
+			version:    "1.3.2-r0",
+			constraint: "<= 1.3.3-r0",
+			satisfied:  true,
+		},
+		{
+			name:       "indirect package no match",
+			version:    "1.3.4-r0",
+			constraint: "<= 1.3.3-r0",
+			satisfied:  false,
+		},
+		{
+			name:       "vulndb fuzzy constraint single quoted",
+			version:    "4.5.2",
+			constraint: "'4.5.1' || '4.5.2'",
+			satisfied:  true,
+		},
+		{
+			name:       "vulndb fuzzy constraint double quoted",
+			version:    "4.5.2",
+			constraint: "\"4.5.1\" || \"4.5.2\"",
+			satisfied:  true,
+		},
+		{
+			name:       "rc candidates with no '-' can match semver pattern",
+			version:    "1.20rc1",
+			constraint: " = 1.20.0-rc1",
+			satisfied:  true,
+		},
+		{
+			name:       "candidates ahead of alpha",
+			version:    "3.11.0",
+			constraint: "> 3.11.0-alpha1",
+			satisfied:  true,
+		},
+		{
+			name:       "candidates ahead of beta",
+			version:    "3.11.0",
+			constraint: "> 3.11.0-beta1",
+			satisfied:  true,
+		},
+		{
+			name:       "candidates ahead of same alpha versions",
+			version:    "3.11.0-alpha5",
+			constraint: "> 3.11.0-alpha1",
+			satisfied:  true,
+		},
+		{
+			name:       "candidates are placed correctly between alpha and release",
+			version:    "3.11.0-beta5",
+			constraint: "3.11.0 || = 3.11.0-alpha1",
+			satisfied:  false,
+		},
+		{
+			name:       "candidates with pre suffix are sorted numerically",
+			version:    "1.0.2pre1",
+			constraint: " < 1.0.2pre2",
+			satisfied:  true,
+		},
+		{
+			name:       "openssl pre2 is still considered less than release",
+			version:    "1.1.1-pre2",
+			constraint: "> 1.1.1-pre1, < 1.1.1",
+			satisfied:  true,
+		},
+		{
+			name:       "major version releases are less than their subsequent patch releases with letter suffixes",
+			version:    "1.1.1",
+			constraint: "> 1.1.1-a",
+			satisfied:  true,
+		},
+		{
+			name:       "date based pep440 version string boundary condition",
+			version:    "2022.12.7",
+			constraint: ">=2017.11.05,<2022.12.07",
+		},
+		{
+			name:       "certifi false positive is fixed",
+			version:    "2022.12.7",
+			constraint: ">=2017.11.05,<2022.12.07",
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			c, err := newPep440Constraint(tc.constraint)
+			require.NoError(t, err)
+			v, err := NewVersion(tc.version, PythonFormat)
+			require.NoError(t, err)
+			sat, err := c.Satisfied(v)
+			require.NoError(t, err)
+			assert.Equal(t, tc.satisfied, sat)
+		})
+	}
+}

grype/version/pep440_version.go

@@ -0,0 +1,34 @@
+package version
+
+import (
+	"fmt"
+
+	goPepVersion "github.com/aquasecurity/go-pep440-version"
+)
+
+var _ Comparator = (*pep440Version)(nil)
+
+type pep440Version struct {
+	obj goPepVersion.Version
+}
+
+func (p pep440Version) Compare(other *Version) (int, error) {
+	if other.Format != PythonFormat {
+		return -1, fmt.Errorf("unable to compare pep440 to given format: %s", other.Format)
+	}
+	if other.rich.pep440version == nil {
+		return -1, fmt.Errorf("given empty pep440 object")
+	}
+
+	return other.rich.pep440version.obj.Compare(p.obj), nil
+}
+
+func newPep440Version(raw string) (pep440Version, error) {
+	parsed, err := goPepVersion.Parse(raw)
+	if err != nil {
+		return pep440Version{}, fmt.Errorf("could not parse pep440 version: %w", err)
+	}
+	return pep440Version{
+		obj: parsed,
+	}, nil
+}

grype/version/version.go

@@ -14,13 +14,14 @@ type Version struct {
 }
 
 type rich struct {
-	cpeVers []cpe.CPE
-	semVer  *semanticVersion
-	apkVer  *apkVersion
-	debVer  *debVersion
-	rpmVer  *rpmVersion
-	kbVer   *kbVersion
-	portVer *portageVersion
+	cpeVers       []cpe.CPE
+	semVer        *semanticVersion
+	apkVer        *apkVersion
+	debVer        *debVersion
+	rpmVer        *rpmVersion
+	kbVer         *kbVersion
+	portVer       *portageVersion
+	pep440version *pep440Version
 }
 
 func NewVersion(raw string, format Format) (*Version, error) {
@@ -66,8 +67,9 @@ func (v *Version) populate() error {
 		v.rich.rpmVer = &ver
 		return err
 	case PythonFormat:
-		// use the fuzzy constraint
-		return nil
+		ver, err := newPep440Version(v.Raw)
+		v.rich.pep440version = &ver
+		return err
 	case KBFormat:
 		ver := newKBVersion(v.Raw)
 		v.rich.kbVer = &ver

test/integration/db_mock_test.go

@@ -100,6 +100,15 @@ func newMockDbStore() *mockStore {
 					},
 				},
 			},
+			"github:language:idris": {
+				"my-package": []grypeDB.Vulnerability{
+					{
+						ID:                "CVE-bogus-my-package-2-idris",
+						VersionConstraint: "< 2.0",
+						VersionFormat:     "unknown",
+					},
+				},
+			},
 			"github:language:javascript": {
 				"npm": []grypeDB.Vulnerability{
 					{
@@ -117,13 +126,6 @@ func newMockDbStore() *mockStore {
 						VersionFormat:     "python",
 					},
 				},
-				"my-package": []grypeDB.Vulnerability{
-					{
-						ID:                "CVE-bogus-my-package-2-python",
-						VersionConstraint: "< 2.0",
-						VersionFormat:     "python",
-					},
-				},
 			},
 			"github:language:ruby": {
 				"bundler": []grypeDB.Vulnerability{

test/integration/match_by_sbom_document_test.go

@@ -55,18 +55,18 @@ func TestMatchBySBOMDocument(t *testing.T) {
 		{
 			name:        "unknown package type",
 			fixture:     "test-fixtures/sbom/syft-sbom-with-unknown-packages.json",
-			expectedIDs: []string{"CVE-bogus-my-package-2-python"},
+			expectedIDs: []string{"CVE-bogus-my-package-2-idris"},
 			expectedDetails: []match.Detail{
 				{
 					Type: match.ExactDirectMatch,
 					SearchedBy: map[string]interface{}{
-						"language":  "python",
-						"namespace": "github:language:python",
+						"language":  "idris",
+						"namespace": "github:language:idris",
 						"package":   map[string]string{"name": "my-package", "version": "1.0.5"},
 					},
 					Found: map[string]interface{}{
-						"versionConstraint": "< 2.0 (python)",
-						"vulnerabilityID":   "CVE-bogus-my-package-2-python",
+						"versionConstraint": "< 2.0 (unknown)",
+						"vulnerabilityID":   "CVE-bogus-my-package-2-idris",
 					},
 					Matcher:    match.StockMatcher,
 					Confidence: 1,

test/integration/test-fixtures/sbom/syft-sbom-with-unknown-packages.json

@@ -4,8 +4,8 @@
       "id": "eeb36c1c-c03a-425b-901f-df918cc3757e",
       "name": "my-package",
       "version": "1.0.5",
-      "type": "binary",
-      "language": "python",
+      "type": "idris",
+      "language": "idris",
       "cpes": [
         "cpe:2.3:a:my-package:my-package:1.0.5:*:*:*:*:*:*:*",
         "cpe:2.3:a:bogus:my-package:1.0.5:*:*:*:*:*:*:*"