Validate debian package version

This patch was originally by Aaron Foster <afoster@cloudflare.com>, as
a local fix at Cloudflare. While we've pushed most of our internal
patches upstream it looks like this one may have been missed. Aaron is
no longer at Cloudflare, so I'm sending this up on his behalf.

This patch simply validates Debian versions so that invalid versions do
not make it into the repo.

I've dug through open and closed PRs, and don't see anything along these
lines, but apologies if I missed it.

I updated the patch for the current `master` branch.

Signed off with internal and external emails for clarity, but commit
is under personal email where we typically do open source contributions
from.

Signed-off-by: Phil Dibowitz <pdibowitz@cloudflare.com>
Signed-off-by: Phil Dibowitz <pdibowitz@ipom.com>

Author: jaymzh

deb/import.go

@@ -124,6 +124,12 @@ func ImportPackageFiles(list *PackageList, packageFiles []string, forceReplace b
 			continue
 		}
 
+		if !isValidVersion(p.Version) {
+			reporter.Warning("Version number ('%s') for the '%s' package is invalid", p.Version, p.Name)
+			failedFiles = append(failedFiles, file)
+			continue
+		}
+
 		if p.Architecture == "" {
 			reporter.Warning("Empty architecture on %s", file)
 			failedFiles = append(failedFiles, file)

deb/package.go

@@ -412,8 +412,9 @@ func versionSatisfiesDependency(version string, dep Dependency) bool {
 		return r == 0
 	case VersionLess:
 		return r < 0
+	// 2 is returned when a package with an invalid version is detected; setting boundary for VersionGreater and GreaterOrEqual cases
 	case VersionGreater:
-		return r > 0
+		return r > 0 && r < 2
 	case VersionLessOrEqual:
 		return r <= 0
 	case VersionGreaterOrEqual:

deb/query.go

@@ -169,10 +169,13 @@ func (q *FieldQuery) Matches(pkg PackageLike) bool {
 		return field != ""
 	case VersionEqual:
 		return CompareVersions(field, q.Value) == 0
+		// 2 is returned when a package with an invalid version is detected; setting boundary for VersionGreater and VersionGreaterOrEqual cases
 	case VersionGreater:
-		return CompareVersions(field, q.Value) > 0
+		result := CompareVersions(field, q.Value)
+		return result > 0 && result < 2
 	case VersionGreaterOrEqual:
-		return CompareVersions(field, q.Value) >= 0
+		result := CompareVersions(field, q.Value)
+		return result >= 0 && result < 2
 	case VersionLess:
 		return CompareVersions(field, q.Value) < 0
 	case VersionLessOrEqual:

deb/version.go

@@ -8,12 +8,26 @@ import (
 	"unicode"
 )
 
+var (
+	upstreamVersionRegex = regexp.MustCompile(`^[0-9][A-Za-z0-9.+~\-]*$`)
+	debianRevisionRegex  = regexp.MustCompile(`^[A-Za-z0-9.+~]*$`)
+)
+
 // Using documentation from: http://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-Version
 
 // CompareVersions compares two package versions
 func CompareVersions(ver1, ver2 string) int {
-	e1, u1, d1 := parseVersion(ver1)
-	e2, u2, d2 := parseVersion(ver2)
+	e1, u1, d1, err := parseVersion(ver1)
+	// if an error is caught during parse, return 2 to signal
+	// an invalid version and handle as needed
+	if err != nil {
+		return 2
+	}
+
+	e2, u2, d2, err := parseVersion(ver2)
+	if err != nil {
+		return 2
+	}
 
 	r := compareVersionPart(e1, e2)
 	if r != 0 {
@@ -29,7 +43,7 @@ func CompareVersions(ver1, ver2 string) int {
 }
 
 // parseVersions breaks down full version to components (possibly empty)
-func parseVersion(ver string) (epoch, upstream, debian string) {
+func parseVersion(ver string) (epoch, upstream, debian string, err error) {
 	i := strings.Index(ver, ":")
 	if i != -1 {
 		epoch, ver = ver[:i], ver[i+1:]
@@ -38,13 +52,55 @@ func parseVersion(ver string) (epoch, upstream, debian string) {
 	i = strings.Index(ver, "-")
 	if i != -1 {
 		debian, ver = ver[i+1:], ver[:i]
+		if debian == "" {
+			// if a hyphen is detected in the upstream version
+			// string without a debian revision following it, the
+			// version is invalid
+			return "", "", "", fmt.Errorf("could not parse version: version string ('%s-') includes hyphen without Debian revision", ver)
+		}
 	}
 
 	upstream = ver
 
 	return
 }
 
+// isValidVersion checks whether package version is compliant with control field spec
+// source: https://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-version
+func isValidVersion(ver string) bool {
+	epoch, upstream, deb, err := parseVersion(ver)
+	if err != nil {
+		return false
+	}
+
+	// validate epoch component
+	if epoch != "" {
+		// uint64 for unexpectedly high epoch values
+		_, err := strconv.ParseUint(epoch, 10, 64)
+		if err != nil {
+			return false
+		}
+	}
+
+	if upstream == "" || !isValidUpstreamVersion(upstream) {
+		return false
+	}
+
+	if deb != "" && !isValidDebianRevision(deb) {
+		return false
+	}
+
+	return true
+}
+
+func isValidUpstreamVersion(upstream string) bool {
+	return upstreamVersionRegex.MatchString(upstream)
+}
+
+func isValidDebianRevision(debian string) bool {
+	return debianRevisionRegex.MatchString(debian)
+}
+
 // compareLexicographic compares in "Debian lexicographic" way, see below compareVersionPart for details
 func compareLexicographic(s1, s2 string) int {
 	i := 0

deb/version_test.go

@@ -10,20 +10,54 @@ type VersionSuite struct {
 var _ = Suite(&VersionSuite{})
 
 func (s *VersionSuite) TestParseVersion(c *C) {
-	e, u, d := parseVersion("1.3.4")
+	e, u, d, err := parseVersion("1.3.4")
 	c.Check([]string{e, u, d}, DeepEquals, []string{"", "1.3.4", ""})
+	c.Check(err, Equals, nil)
 
-	e, u, d = parseVersion("4:1.3:4")
+	e, u, d, err = parseVersion("4:1.3:4")
 	c.Check([]string{e, u, d}, DeepEquals, []string{"4", "1.3:4", ""})
+	c.Check(err, Equals, nil)
 
-	e, u, d = parseVersion("1.3.4-1")
+	e, u, d, err = parseVersion("1.3.4-1")
 	c.Check([]string{e, u, d}, DeepEquals, []string{"", "1.3.4", "1"})
+	c.Check(err, Equals, nil)
 
-	e, u, d = parseVersion("1.3-pre4-1")
+	e, u, d, err = parseVersion("1.3-pre4-1")
 	c.Check([]string{e, u, d}, DeepEquals, []string{"", "1.3", "pre4-1"})
+	c.Check(err, Equals, nil)
 
-	e, u, d = parseVersion("4:1.3-pre4-1")
+	e, u, d, err = parseVersion("4:1.3-pre4-1")
 	c.Check([]string{e, u, d}, DeepEquals, []string{"4", "1.3", "pre4-1"})
+	c.Check(err, Equals, nil)
+
+	e, u, d, err = parseVersion("1:1.2024-")
+	c.Check([]string{e, u, d}, DeepEquals, []string{"", "", ""})
+	c.Check(err.Error(), Equals, "could not parse version: version string ('1.2024-') includes hyphen without Debian revision")
+}
+
+func (s *VersionSuite) TestIsValidVersion(c *C) {
+	// valid cases
+	valid := isValidVersion("1.2.3-abc")
+	c.Check(valid, Equals, true)
+
+	valid = isValidVersion("1.0.1337~rc2-3")
+	c.Check(valid, Equals, true)
+
+	valid = isValidVersion("1.2.3+fdsfgs")
+	c.Check(valid, Equals, true)
+
+	valid = isValidVersion("1:2.3~4-5six")
+	c.Check(valid, Equals, true)
+
+	// invalid cases
+	valid = isValidVersion("1:1.2.3-")
+	c.Check(valid, Equals, false)
+
+	valid = isValidVersion("42:")
+	c.Check(valid, Equals, false)
+
+	valid = isValidVersion("1:a.1.2.3~-4")
+	c.Check(valid, Equals, false)
 }
 
 func (s *VersionSuite) TestCompareLexicographic(c *C) {
@@ -101,6 +135,10 @@ func (s *VersionSuite) TestCompareVersions(c *C) {
 
 	c.Check(CompareVersions("5.2.0.3", "5.2.0.283"), Equals, -1)
 	c.Check(CompareVersions("4.3.5a", "4.3.5-rc3-1"), Equals, 1)
+
+	// version validation happens independent of CompareVersions, so only testing the
+	// edge case where a package is missing a Debian version during parseVersion
+	c.Check(CompareVersions("1:abc~1.2.3-", "1:1.2.3~abc-good"), Equals, 2)
 }
 
 func (s *VersionSuite) TestParseDependency(c *C) {