Fix crash when loading buildpack from OCI archive with relative paths

Signed-off-by: Pavel Busko <pavel.busko@sap.com> Signed-off-by: Philipp Stehle <philipp.stehle@sap.com> Co-authored-by: Philipp Stehle <philipp.stehle@sap.com>
buildpacks · Sep 2, 2022 · 75a0d6b · 75a0d6b
1 parent 651b9a7
commit 75a0d6b
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 4 deletions.
diff --git a/internal/paths/paths.go b/internal/paths/paths.go
@@ -3,6 +3,7 @@ package paths
 import (
 	"net/url"
 	"os"
+	"path"
 	"path/filepath"
 	"regexp"
 	"runtime"
@@ -133,3 +134,8 @@ func WindowsPathSID(uid, gid int) string {
 	}
 	return "S-1-5-32-545" // BUILTIN\Users
 }
+
+// CanonicalTarPath return a cleaned path (see path.Clean) with leading slashes removed
+func CanonicalTarPath(p string) string {
+	return strings.TrimPrefix(path.Clean(p), "/")
+}
diff --git a/internal/paths/paths_test.go b/internal/paths/paths_test.go
@@ -259,4 +259,41 @@ func testPaths(t *testing.T, when spec.G, it spec.S) {
 			})
 		})
 	})
+
+	when("#CanonicalTarPath", func() {
+		for _, params := range []struct {
+			desc     string
+			path     string
+			expected string
+		}{
+			{
+				desc:     "noop",
+				path:     "my/clean/path",
+				expected: "my/clean/path",
+			},
+			{
+				desc:     "leading slash",
+				path:     "/my/path",
+				expected: "my/path",
+			},
+			{
+				desc:     "dot",
+				path:     "my/./path",
+				expected: "my/path",
+			},
+			{
+				desc:     "dotdot",
+				path:     "my/../my/path",
+				expected: "my/path",
+			},
+		} {
+			params := params
+
+			when(params.desc+":"+params.path, func() {
+				it(fmt.Sprintf("returns %v", params.expected), func() {
+					h.AssertEq(t, paths.CanonicalTarPath(params.path), params.expected)
+				})
+			})
+		}
+	})
 }
diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go
@@ -7,12 +7,13 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
-	"path"
 	"path/filepath"
 	"time"
 
 	"github.com/docker/docker/pkg/ioutils"
 	"github.com/pkg/errors"
+
+	"github.com/buildpacks/pack/internal/paths"
 )
 
 var NormalizedDateTime time.Time
@@ -139,6 +140,7 @@ func IsEntryNotExist(err error) bool {
 
 // ReadTarEntry reads and returns a tar file
 func ReadTarEntry(rc io.Reader, entryPath string) (*tar.Header, []byte, error) {
+	canonicalEntryPath := paths.CanonicalTarPath(entryPath)
 	tr := tar.NewReader(rc)
 	for {
 		header, err := tr.Next()
@@ -149,7 +151,7 @@ func ReadTarEntry(rc io.Reader, entryPath string) (*tar.Header, []byte, error) {
 			return nil, nil, errors.Wrap(err, "failed to get next tar entry")
 		}
 
-		if path.Clean(header.Name) == entryPath {
+		if paths.CanonicalTarPath(header.Name) == canonicalEntryPath {
 			buf, err := ioutil.ReadAll(tr)
 			if err != nil {
 				return nil, nil, errors.Wrapf(err, "failed to read contents of '%s'", entryPath)

diff --git a/pkg/buildpack/oci_layout_package.go b/pkg/buildpack/oci_layout_package.go
@@ -12,6 +12,7 @@ import (
 	v1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 
+	"github.com/buildpacks/pack/internal/paths"
 	"github.com/buildpacks/pack/internal/style"
 	"github.com/buildpacks/pack/pkg/archive"
 	blob2 "github.com/buildpacks/pack/pkg/blob"
@@ -125,7 +126,7 @@ func (o *ociLayoutPackage) GetLayer(diffID string) (io.ReadCloser, error) {
 	}
 
 	layerDescriptor := o.manifest.Layers[index]
-	layerPath := pathFromDescriptor(layerDescriptor)
+	layerPath := paths.CanonicalTarPath(pathFromDescriptor(layerDescriptor))
 
 	blobReader, err := o.blob.Open()
 	if err != nil {
@@ -142,7 +143,7 @@ func (o *ociLayoutPackage) GetLayer(diffID string) (io.ReadCloser, error) {
 			return nil, errors.Wrap(err, "failed to get next tar entry")
 		}
 
-		if path.Clean(header.Name) == path.Clean(layerPath) {
+		if paths.CanonicalTarPath(header.Name) == layerPath {
 			finalReader := blobReader
 
 			if strings.HasSuffix(layerDescriptor.MediaType, ".gzip") {