CVE-2020-7667: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: In package github.com/sassoftware/go-rpmutils/cpio before version 0.1.0, the CPIO extraction functionality doesn't sanitize the paths of the archived files for leading and non-leading ".." which leads in file extraction outside of the current directory. Note: the fixing commit was applied to all affected versions which were re-released.
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: 7.5
Published: 2020-06-24T12:15Z
Repository: https://github.com/sassoftware/go-rpmutils
Language: Go
Files changed: cpio/extract.go, cpio/extract_test.go, rpmutils_test.go
Commit date: 2020-06-12 11:22:45 -0400
Commit message: Prevent cpio.Extract from writing into parent directories

Diff: diff --git a/cpio/extract.go b/cpio/extract.go
index 2b753bf..b907ae1 100644
--- a/cpio/extract.go
+++ b/cpio/extract.go
@@ -19,10 +19,12 @@ package cpio
 import (
 	"fmt"
 	"io"
 	"os"
 	"path"
+	"path/filepath"
+	"strings"
 
 	"github.com/sassoftware/go-rpmutils/fileutil"
 )
 
 // Standard set of permission bit masks.
@@ -37,11 +39,13 @@ const (
 	S_ISBLK  = 060000  // Block special file
 	S_ISCHR  = 020000  // Character special file
 	S_ISSOCK = 0140000 // Socket
 )
 
+// Extract the contents of a cpio stream from r to the destination directory dest
 func Extract(rs io.Reader, dest string) error {
+	dest = filepath.Clean(filepath.FromSlash(dest))
 	linkMap := make(map[int][]string)
 
 	stream := NewCpioStream(rs)
 
 	for {
@@ -52,20 +56,25 @@ func Extract(rs io.Reader, dest string) error {
 
 		if entry.Header.filename == TRAILER {
 			break
 		}
 
-		target := path.Join(dest, path.Clean(entry.Header.filename))
-		parent := path.Dir(target)
-
+		// sanitize path
+		target := path.Clean(entry.Header.filename)
+		for strings.HasPrefix(target, "../") {
+			target = target[3:]
+		}
+		target = filepath.Join(dest, filepath.FromSlash(target))
+		if !strings.HasPrefix(target, dest+string(filepath.Separator)) && dest != target {
+			// this shouldn't happen due to the sanitization above but always check
+			return fmt.Errorf("invalid cpio path %q", entry.Header.filename)
+		}
 		// Create the parent directory if it doesn't exist.
-		if _, err := os.Stat(parent); os.IsNotExist(err) {
-			if err := os.MkdirAll(parent, 0755); err != nil {
-				return err
-			}
+		parent := filepath.Dir(target)
+		if err := os.MkdirAll(parent, 0755); err != nil {
+			return err
 		}
-
 		// FIXME: Need a makedev implementation in go.
 
 		switch entry.Header.Mode() &^ 07777 {
 		case S_ISCHR:
 			logger.Debug("unpacking char device")
diff --git a/cpio/extract_test.go b/cpio/extract_test.go
index c2c4bbe..bd89440 100644
--- a/cpio/extract_test.go
+++ b/cpio/extract_test.go
@@ -17,10 +17,11 @@
 package cpio
 
 import (
 	"io/ioutil"
 	"os"
+	"path/filepath"
 	"testing"
 	"testing/iotest"
 
 	"github.com/op/go-logging"
 )
@@ -56,10 +57,31 @@ func TestExtract(t *testing.T) {
 	if err := Extract(hf, tmpdir); err != nil {
 		t.Fatal(err)
 	}
 }
 
+func TestExtractDotdot(t *testing.T) {
+	setupLogging()
+	f, err := os.Open("../testdata/dotdot.cpio")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer f.Close()
+	tmpdir, err := ioutil.TempDir("", "cpio")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpdir)
+	err = Extract(f, tmpdir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := os.Stat(filepath.Join(tmpdir, "aaaaaaaaa")); err != nil {
+		t.Error("expected file with ../ to extract into top of destdir:", err)
+	}
+}
+
 var _format = logging.MustStringFormatter(
 	`%{color}%{time:15:04:05.000} %{shortfunc} ▶ %{level:.4s} %{id:03x}%{color:reset} %{message}`,
 )
 
 func setupLogging() {
diff --git a/rpmutils_test.go b/rpmutils_test.go
index 5968bf2..9c4d82c 100644
--- a/rpmutils_test.go
+++ b/rpmutils_test.go
@@ -26,10 +26,11 @@ import (
 func TestReadHeader(t *testing.T) {
 	f, err := os.Open("./testdata/simple-1.0.1-1.i386.rpm")
 	if err != nil {
 		t.Fatal(err)
 	}
+	defer f.Close()
 
 	hdr, err := ReadHeader(iotest.HalfReader(f))
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -60,10 +61,11 @@ func TestReadHeader(t *testing.T) {
 func TestPayloadReader(t *testing.T) {
 	f, err := os.Open("./testdata/simple-1.0.1-1.i386.rpm")
 	if err != nil {
 		t.Fatal(err)
 	}
+	defer f.Close()
 
 	rpm, err := ReadRpm(iotest.HalfReader(f))
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -90,20 +92,22 @@ func TestPayloadReader(t *testing.T) {
 func TestExpandPayload(t *testing.T) {
 	f, err := os.Open("./testdata/simple-1.0.1-1.i386.rpm")
 	if err != nil {
 		t.Fatal(err)
 	}
+	defer f.Close()
 
 	rpm, err := ReadRpm(iotest.HalfReader(f))
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	tmpdir, err := ioutil.TempDir("", "rpmutil")
 	if err != nil {
 		t.Fatal(err)
 	}
+	defer os.RemoveAll(tmpdir)
 
 	if err := rpm.ExpandPayload(tmpdir); err != nil {
 		t.Fatal(err)
 	}
 }
diff --git a/testdata/dotdot.cpio b/testdata/dotdot.cpio
new file mode 100644
index 0000000..be2d3a7
Binary files /dev/null and b/testdata/dotdot.cpio differ

Vulnerable code: 		target := path.Join(dest, path.Clean(entry.Header.filename))
		parent := path.Dir(target)

		if _, err := os.Stat(parent); os.IsNotExist(err) {
			if err := os.MkdirAll(parent, 0755); err != nil {
				return err
			}

Fixed code: 	"path/filepath"
	"strings"
// Extract the contents of a cpio stream from r to the destination directory dest
	dest = filepath.Clean(filepath.FromSlash(dest))
		// sanitize path
		target := path.Clean(entry.Header.filename)
		for strings.HasPrefix(target, "../") {
			target = target[3:]
		}
		target = filepath.Join(dest, filepath.FromSlash(target))
		if !strings.HasPrefix(target, dest+string(filepath.Separator)) && dest != target {
			// this shouldn't happen due to the sanitization above but always check
			return fmt.Errorf("invalid cpio path %q", entry.Header.filename)
		}
		parent := filepath.Dir(target)
		if err := os.MkdirAll(parent, 0755); err != nil {
			return err
	"path/filepath"
func TestExtractDotdot(t *testing.T) {
	setupLogging()
	f, err := os.Open("../testdata/dotdot.cpio")
	if err != nil {
		t.Fatal(err)
	}
	defer f.Close()
	tmpdir, err := ioutil.TempDir("", "cpio")
	if err != nil {
		t.Fatal(err)
	}
	defer os.RemoveAll(tmpdir)
	err = Extract(f, tmpdir)
	if err != nil {
		t.Fatal(err)
	}
	if _, err := os.Stat(filepath.Join(tmpdir, "aaaaaaaaa")); err != nil {
		t.Error("expected file with ../ to extract into top of destdir:", err)
	}
}

	defer f.Close()
	defer f.Close()
	defer f.Close()
	defer os.RemoveAll(tmpdir)