CVE-2021-3762: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: A directory traversal vulnerability was found in the ClairCore engine of Clair. An attacker can exploit this by supplying a crafted container image which, when scanned by Clair, allows for arbitrary file write on the filesystem, potentially allowing for remote code execution.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 9.8
Published: 2022-03-03T22:15Z
Repository: https://github.com/quay/claircore
Language: Go
Files changed: rpm/packagescanner.go, rpm/packagescanner_test.go
Commit date: 2021-09-28 09:38:13 -0500
Commit message: rpm: prevent directory traversal when extracting

A malicious tar archive with a member consisting of multiple "parent"
elements could cause a file outside of the target directory to be
overwritten.

Fixes: CVE-2021-3762
Signed-off-by: Hank Donnay <hdonnay@redhat.com>

Diff: diff --git a/rpm/packagescanner.go b/rpm/packagescanner.go
index 19e3bfe..6d0bcec 100644
--- a/rpm/packagescanner.go
+++ b/rpm/packagescanner.go
@@ -168,11 +168,11 @@ func (ps *Scanner) Scan(ctx context.Context, layer *claircore.Layer) ([]*clairco
 			// Whiteout, skip.
 			stats.Whiteout++
 			continue
 		}
 		// Build the path on the filesystem.
-		tgt := filepath.Join(root, filepath.Clean(h.Name))
+		tgt := relPath(root, h.Name)
 		// Since tar, as a format, doesn't impose ordering requirements, make
 		// sure to create all parent directories of the current entry.
 		d := filepath.Dir(tgt)
 		if _, ok := made[d]; !ok {
 			if err := os.MkdirAll(d, dirMode); err != nil {
@@ -210,16 +210,16 @@ func (ps *Scanner) Scan(ctx context.Context, layer *claircore.Layer) ([]*clairco
 				zlog.Warn(ctx).Err(err).Msg("error closing new file")
 			}
 			stats.Reg++
 		case tar.TypeSymlink:
 			// Normalize the link target into the root.
-			ln := filepath.Join(root, filepath.Clean(h.Linkname))
+			ln := relPath(root, h.Linkname)
 			err = os.Symlink(ln, tgt)
 			stats.Symlink++
 		case tar.TypeLink:
 			// Normalize the link target into the root.
-			ln := filepath.Join(root, filepath.Clean(h.Linkname))
+			ln := relPath(root, h.Linkname)
 			_, exists := os.Lstat(ln)
 			switch {
 			case errors.Is(exists, nil):
 				err = os.Link(ln, tgt)
 			case errors.Is(exists, os.ErrNotExist):
@@ -467,5 +467,13 @@ func checkMagic(ctx context.Context, r io.Reader) bool {
 		}
 	}
 
 	return false
 }
+
+// RelPath takes a member and forcibly interprets it as a path underneath root.
+//
+// This should be used anytime a path for a new file on disk is needed when
+// unpacking a tar.
+func relPath(root, member string) string {
+	return filepath.Join(root, filepath.Join("/", member))
+}
diff --git a/rpm/packagescanner_test.go b/rpm/packagescanner_test.go
index fed6472..a97dce7 100644
--- a/rpm/packagescanner_test.go
+++ b/rpm/packagescanner_test.go
@@ -4,10 +4,11 @@ import (
 	"context"
 	"net/http"
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/quay/zlog"
@@ -1634,5 +1635,41 @@ func TestScan(t *testing.T) {
 	}
 	if got, want := ms, prevs; !cmp.Equal(got, want) {
 		t.Error(cmp.Diff(got, want))
 	}
 }
+
+func TestRelPath(t *testing.T) {
+	root := `/tmp/fakedata`
+	tt := [][2]string{
+		{
+			"dev/null",
+			"/dev/null",
+		},
+		{
+			"dev/null",
+			"./dev/null",
+		},
+		{
+			"dev/null",
+			"dev/null",
+		},
+		{
+			"dev/null",
+			strings.Repeat("../", 10) + "dev/null",
+		},
+		{
+			"dev/null",
+			strings.Repeat("../", 10) + "dev/./../dev/null",
+		},
+	}
+
+	for _, tc := range tt {
+		want := filepath.Join(root, tc[0])
+		t.Logf("in: %q + %q", root, tc[1])
+		got := relPath(root, tc[1])
+		t.Logf("got: %q, want: %q", got, want)
+		if got != want {
+			t.Error()
+		}
+	}
+}

Vulnerable code: 		tgt := filepath.Join(root, filepath.Clean(h.Name))
			ln := filepath.Join(root, filepath.Clean(h.Linkname))
			ln := filepath.Join(root, filepath.Clean(h.Linkname))
Fixed code: 		tgt := relPath(root, h.Name)
			ln := relPath(root, h.Linkname)
			ln := relPath(root, h.Linkname)

// RelPath takes a member and forcibly interprets it as a path underneath root.
//
// This should be used anytime a path for a new file on disk is needed when
// unpacking a tar.
func relPath(root, member string) string {
	return filepath.Join(root, filepath.Join("/", member))
}
	"strings"

func TestRelPath(t *testing.T) {
	root := `/tmp/fakedata`
	tt := [][2]string{
		{
			"dev/null",
			"/dev/null",
		},
		{
			"dev/null",
			"./dev/null",
		},
		{
			"dev/null",
			"dev/null",
		},
		{
			"dev/null",
			strings.Repeat("../", 10) + "dev/null",
		},
		{
			"dev/null",
			strings.Repeat("../", 10) + "dev/./../dev/null",
		},
	}

	for _, tc := range tt {
		want := filepath.Join(root, tc[0])
		t.Logf("in: %q + %q", root, tc[1])
		got := relPath(root, tc[1])
		t.Logf("got: %q, want: %q", got, want)
		if got != want {
			t.Error()
		}
	}
}