CVE-2021-21272: Improper Link Resolution Before File Access ('Link Following') (CWE-59)
value: ORAS is open source software which enables a way to push OCI Artifacts to OCI Conformant registries. ORAS is both a CLI for initial testing and a Go Module. In ORAS from version 0.4.0 and before version 0.9.0, there is a "zip-slip" vulnerability. The directory support feature allows the downloaded gzipped tarballs to be automatically extracted to the user-specified directory where the tarball can have symbolic links and hard links. A well-crafted tarball or tarballs allow malicious artifact providers linking, writing, or overwriting specific files on the host filesystem outside of the user-specified directory unexpectedly with the same permissions as the user who runs `oras pull`. Users of the affected versions are impacted if they are `oras` CLI users who runs `oras pull`, or if they are Go programs, which invoke `github.com/deislabs/oras/pkg/content.FileStore`. The problem has been fixed in version 0.9.0. For `oras` CLI users, there is no workarounds other than pulling from a trusted artifact provider. For `oras` package users, the workaround is to not use `github.com/deislabs/oras/pkg/content.FileStore`, and use other content stores instead, or pull from a trusted artifact provider.
CWE description: The product attempts to access a file based on the filename, but it does not properly prevent that filename from identifying a link or shortcut that resolves to an unintended resource.
CVSS v3 base score: 7.7
Published: 2021-01-25T19:15Z
Repository: https://github.com/deislabs/oras
Language: Go
Files changed: pkg/content/utils.go, pkg/oras/oras_test.go
Commit date: 2021-01-22 10:26:41 -0500
Commit message: Merge pull request from GHSA-g5v4-5x39-vwhx

* check hard link

* no following symbolic link

* bug fix

* add initial test to reproduce ghsa-g5v4-5x39-vwhx

Signed-off-by: jdolitsky <393494+jdolitsky@users.noreply.github.com>

* fix test for symbolic link

* fix bug

* add test for hardlink

Signed-off-by: jdolitsky <393494+jdolitsky@users.noreply.github.com>

* catch the parent folder

* remove check for hard link for consistency

* remove unncessary test for hard links

* Revert "remove unncessary test for hard links"

This reverts commit b3136611810f49074dfc6aef158b3d24466d2ed9.

* Revert "remove check for hard link for consistency"

This reverts commit d7b7346598c92ff9c430a42763d810b34d3f1ac2.

* check links for all link types

* add tests

Co-authored-by: jdolitsky <393494+jdolitsky@users.noreply.github.com>
Diff: diff --git a/pkg/content/utils.go b/pkg/content/utils.go
index d4095d1..656dee2 100644
--- a/pkg/content/utils.go
+++ b/pkg/content/utils.go
@@ -99,19 +99,28 @@ func extractTarDirectory(root, prefix string, r io.Reader) error {
 			return err
 		}
 
 		// Name check
 		name := header.Name
-		path, err := filepath.Rel(prefix, name)
+		path, err := ensureBasePath(root, prefix, name)
 		if err != nil {
 			return err
 		}
-		if strings.HasPrefix(path, "../") {
-			return fmt.Errorf("%q does not have prefix %q", name, prefix)
-		}
 		path = filepath.Join(root, path)
 
+		// Link check
+		switch header.Typeflag {
+		case tar.TypeLink, tar.TypeSymlink:
+			link := header.Linkname
+			if !filepath.IsAbs(link) {
+				link = filepath.Join(filepath.Dir(name), link)
+			}
+			if _, err := ensureBasePath(root, prefix, link); err != nil {
+				return err
+			}
+		}
+
 		// Create content
 		switch header.Typeflag {
 		case tar.TypeReg:
 			err = writeFile(path, tr, header.FileInfo().Mode())
 		case tar.TypeDir:
@@ -130,10 +139,38 @@ func extractTarDirectory(root, prefix string, r io.Reader) error {
 		// Change access time and modification time if possible (error ignored)
 		os.Chtimes(path, header.AccessTime, header.ModTime)
 	}
 }
 
+// ensureBasePath ensures the target path is in the base path,
+// returning its relative path to the base path.
+func ensureBasePath(root, base, target string) (string, error) {
+	path, err := filepath.Rel(base, target)
+	if err != nil {
+		return "", err
+	}
+	cleanPath := filepath.ToSlash(filepath.Clean(path))
+	if cleanPath == ".." || strings.HasPrefix(cleanPath, "../") {
+		return "", fmt.Errorf("%q is outside of %q", target, base)
+	}
+
+	// No symbolic link allowed in the relative path
+	dir := filepath.Dir(path)
+	for dir != "." {
+		if info, err := os.Lstat(filepath.Join(root, dir)); err != nil {
+			if !os.IsNotExist(err) {
+				return "", err
+			}
+		} else if info.Mode()&os.ModeSymlink != 0 {
+			return "", fmt.Errorf("no symbolic link allowed between %q and %q", base, target)
+		}
+		dir = filepath.Dir(dir)
+	}
+
+	return path, nil
+}
+
 func writeFile(path string, r io.Reader, perm os.FileMode) error {
 	file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, perm)
 	if err != nil {
 		return err
 	}
diff --git a/pkg/oras/oras_test.go b/pkg/oras/oras_test.go
index 076c14a..a611ab6 100644
--- a/pkg/oras/oras_test.go
+++ b/pkg/oras/oras_test.go
@@ -1,10 +1,15 @@
 package oras
 
 import (
+	"archive/tar"
+	"bytes"
+	"compress/gzip"
 	"context"
+	_ "crypto/sha256"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
 	"testing"
 	"time"
@@ -15,10 +20,11 @@ import (
 	"github.com/containerd/containerd/remotes"
 	"github.com/containerd/containerd/remotes/docker"
 	"github.com/docker/distribution/configuration"
 	"github.com/docker/distribution/registry"
 	_ "github.com/docker/distribution/registry/storage/driver/inmemory"
+	digest "github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/phayes/freeport"
 	"github.com/stretchr/testify/suite"
 )
 
@@ -311,8 +317,131 @@ func (suite *ORASTestSuite) Test_3_Conditional_Pull() {
 		content := []byte(data[1])
 		suite.Equal(content, actualContent, "test content matches on pull")
 	}
 }
 
+// Test for vulnerability GHSA-g5v4-5x39-vwhx
+func (suite *ORASTestSuite) Test_4_GHSA_g5v4_5x39_vwhx() {
+	var testVulnerability = func(headers []tar.Header, tag string, expectedError string) {
+		// Step 1: build malicious tar+gzip
+		buf := bytes.NewBuffer(nil)
+		digester := digest.Canonical.Digester()
+		zw := gzip.NewWriter(io.MultiWriter(buf, digester.Hash()))
+		tarDigester := digest.Canonical.Digester()
+		tw := tar.NewWriter(io.MultiWriter(zw, tarDigester.Hash()))
+		for _, header := range headers {
+			err := tw.WriteHeader(&header)
+			suite.Nil(err, "error writing header")
+		}
+		err := tw.Close()
+		suite.Nil(err, "error closing tar")
+		err = zw.Close()
+		suite.Nil(err, "error closing gzip")
+
+		// Step 2: construct malicious descriptor
+		evilDesc := ocispec.Descriptor{
+			MediaType: ocispec.MediaTypeImageLayerGzip,
+			Digest:    digester.Digest(),
+			Size:      int64(buf.Len()),
+			Annotations: map[string]string{
+				orascontent.AnnotationDigest: tarDigester.Digest().String(),
+				orascontent.AnnotationUnpack: "true",
+				ocispec.AnnotationTitle:      "foo",
+			},
+		}
+
+		// Step 3: upload malicious artifact to registry
+		memoryStore := orascontent.NewMemoryStore()
+		memoryStore.Set(evilDesc, buf.Bytes())
+		ref := fmt.Sprintf("%s/evil:%s", suite.DockerRegistryHost, tag)
+		_, err = Push(newContext(), newResolver(), ref, memoryStore, []ocispec.Descriptor{evilDesc})
+		suite.Nil(err, "no error pushing test data")
+
+		// Step 4: pull malicious tar with oras filestore and ensure error
+		tempDir, err := ioutil.TempDir("", "oras_test")
+		if err != nil {
+			suite.FailNow("error creating temp directory", err)
+		}
+		defer os.RemoveAll(tempDir)
+		store := orascontent.NewFileStore(tempDir)
+		defer store.Close()
+		ref = fmt.Sprintf("%s/evil:%s", suite.DockerRegistryHost, tag)
+		_, _, err = Pull(newContext(), newResolver(), ref, store)
+		suite.NotNil(err, "error expected pulling malicious tar")
+		suite.Contains(err.Error(),
+			expectedError,
+			"did not get correct error message",
+		)
+	}
+
+	tests := []struct {
+		name          string
+		headers       []tar.Header
+		tag           string
+		expectedError string
+	}{
+		{
+			name: "Test symbolic link path traversal",
+			headers: []tar.Header{
+				{
+					Typeflag: tar.TypeDir,
+					Name:     "foo/subdir/",
+					Mode:     0755,
+				},
+				{ // Symbolic link to `foo`
+					Typeflag: tar.TypeSymlink,
+					Name:     "foo/subdir/parent",
+					Linkname: "..",
+					Mode:     0755,
+				},
+				{ // Symbolic link to `../etc/passwd`
+					Typeflag: tar.TypeSymlink,
+					Name:     "foo/subdir/parent/passwd",
+					Linkname: "../../etc/passwd",
+					Mode:     0644,
+				},
+				{ // Symbolic link to `../etc`
+					Typeflag: tar.TypeSymlink,
+					Name:     "foo/subdir/parent/etc",
+					Linkname: "../../etc",
+					Mode:     0644,
+				},
+			},
+			tag:           "symlink_path",
+			expectedError: "no symbolic link allowed",
+		},
+		{
+			name: "Test symbolic link pointing to outside",
+			headers: []tar.Header{
+				{ // Symbolic link to `/etc/passwd`
+					Typeflag: tar.TypeSymlink,
+					Name:     "foo/passwd",
+					Linkname: "../../../etc/passwd",
+					Mode:     0644,
+				},
+			},
+			tag:           "symlink",
+			expectedError: "is outside of",
+		},
+		{
+			name: "Test hard link pointing to outside",
+			headers: []tar.Header{
+				{ // Hard link to `/etc/passwd`
+					Typeflag: tar.TypeLink,
+					Name:     "foo/passwd",
+					Linkname: "../../../etc/passwd",
+					Mode:     0644,
+				},
+			},
+			tag:           "hardlink",
+			expectedError: "is outside of",
+		},
+	}
+	for _, test := range tests {
+		suite.T().Log(test.name)
+		testVulnerability(test.headers, test.tag, test.expectedError)
+	}
+}
+
 func TestORASTestSuite(t *testing.T) {
 	suite.Run(t, new(ORASTestSuite))
 }

Vulnerable code: 		path, err := filepath.Rel(prefix, name)
		if strings.HasPrefix(path, "../") {
			return fmt.Errorf("%q does not have prefix %q", name, prefix)
		}
Fixed code: 		path, err := ensureBasePath(root, prefix, name)
		// Link check
		switch header.Typeflag {
		case tar.TypeLink, tar.TypeSymlink:
			link := header.Linkname
			if !filepath.IsAbs(link) {
				link = filepath.Join(filepath.Dir(name), link)
			}
			if _, err := ensureBasePath(root, prefix, link); err != nil {
				return err
			}
		}

// ensureBasePath ensures the target path is in the base path,
// returning its relative path to the base path.
func ensureBasePath(root, base, target string) (string, error) {
	path, err := filepath.Rel(base, target)
	if err != nil {
		return "", err
	}
	cleanPath := filepath.ToSlash(filepath.Clean(path))
	if cleanPath == ".." || strings.HasPrefix(cleanPath, "../") {
		return "", fmt.Errorf("%q is outside of %q", target, base)
	}

	// No symbolic link allowed in the relative path
	dir := filepath.Dir(path)
	for dir != "." {
		if info, err := os.Lstat(filepath.Join(root, dir)); err != nil {
			if !os.IsNotExist(err) {
				return "", err
			}
		} else if info.Mode()&os.ModeSymlink != 0 {
			return "", fmt.Errorf("no symbolic link allowed between %q and %q", base, target)
		}
		dir = filepath.Dir(dir)
	}

	return path, nil
}

	"archive/tar"
	"bytes"
	"compress/gzip"
	_ "crypto/sha256"
	"io"
	digest "github.com/opencontainers/go-digest"
// Test for vulnerability GHSA-g5v4-5x39-vwhx
func (suite *ORASTestSuite) Test_4_GHSA_g5v4_5x39_vwhx() {
	var testVulnerability = func(headers []tar.Header, tag string, expectedError string) {
		// Step 1: build malicious tar+gzip
		buf := bytes.NewBuffer(nil)
		digester := digest.Canonical.Digester()
		zw := gzip.NewWriter(io.MultiWriter(buf, digester.Hash()))
		tarDigester := digest.Canonical.Digester()
		tw := tar.NewWriter(io.MultiWriter(zw, tarDigester.Hash()))
		for _, header := range headers {
			err := tw.WriteHeader(&header)
			suite.Nil(err, "error writing header")
		}
		err := tw.Close()
		suite.Nil(err, "error closing tar")
		err = zw.Close()
		suite.Nil(err, "error closing gzip")

		// Step 2: construct malicious descriptor
		evilDesc := ocispec.Descriptor{
			MediaType: ocispec.MediaTypeImageLayerGzip,
			Digest:    digester.Digest(),
			Size:      int64(buf.Len()),
			Annotations: map[string]string{
				orascontent.AnnotationDigest: tarDigester.Digest().String(),
				orascontent.AnnotationUnpack: "true",
				ocispec.AnnotationTitle:      "foo",
			},
		}

		// Step 3: upload malicious artifact to registry
		memoryStore := orascontent.NewMemoryStore()
		memoryStore.Set(evilDesc, buf.Bytes())
		ref := fmt.Sprintf("%s/evil:%s", suite.DockerRegistryHost, tag)
		_, err = Push(newContext(), newResolver(), ref, memoryStore, []ocispec.Descriptor{evilDesc})
		suite.Nil(err, "no error pushing test data")

		// Step 4: pull malicious tar with oras filestore and ensure error
		tempDir, err := ioutil.TempDir("", "oras_test")
		if err != nil {
			suite.FailNow("error creating temp directory", err)
		}
		defer os.RemoveAll(tempDir)
		store := orascontent.NewFileStore(tempDir)
		defer store.Close()
		ref = fmt.Sprintf("%s/evil:%s", suite.DockerRegistryHost, tag)
		_, _, err = Pull(newContext(), newResolver(), ref, store)
		suite.NotNil(err, "error expected pulling malicious tar")
		suite.Contains(err.Error(),
			expectedError,
			"did not get correct error message",
		)
	}

	tests := []struct {
		name          string
		headers       []tar.Header
		tag           string
		expectedError string
	}{
		{
			name: "Test symbolic link path traversal",
			headers: []tar.Header{
				{
					Typeflag: tar.TypeDir,
					Name:     "foo/subdir/",
					Mode:     0755,
				},
				{ // Symbolic link to `foo`
					Typeflag: tar.TypeSymlink,
					Name:     "foo/subdir/parent",
					Linkname: "..",
					Mode:     0755,
				},
				{ // Symbolic link to `../etc/passwd`
					Typeflag: tar.TypeSymlink,
					Name:     "foo/subdir/parent/passwd",
					Linkname: "../../etc/passwd",
					Mode:     0644,
				},
				{ // Symbolic link to `../etc`
					Typeflag: tar.TypeSymlink,
					Name:     "foo/subdir/parent/etc",
					Linkname: "../../etc",
					Mode:     0644,
				},
			},
			tag:           "symlink_path",
			expectedError: "no symbolic link allowed",
		},
		{
			name: "Test symbolic link pointing to outside",
			headers: []tar.Header{
				{ // Symbolic link to `/etc/passwd`
					Typeflag: tar.TypeSymlink,
					Name:     "foo/passwd",
					Linkname: "../../../etc/passwd",
					Mode:     0644,
				},
			},
			tag:           "symlink",
			expectedError: "is outside of",
		},
		{
			name: "Test hard link pointing to outside",
			headers: []tar.Header{
				{ // Hard link to `/etc/passwd`
					Typeflag: tar.TypeLink,
					Name:     "foo/passwd",
					Linkname: "../../../etc/passwd",
					Mode:     0644,
				},
			},
			tag:           "hardlink",
			expectedError: "is outside of",
		},
	}
	for _, test := range tests {
		suite.T().Log(test.name)
		testVulnerability(test.headers, test.tag, test.expectedError)
	}
}