diff --git a/pkg/oci/containerize.go b/pkg/oci/containerize.go index 8f1041c127..83045b64fe 100644 --- a/pkg/oci/containerize.go +++ b/pkg/oci/containerize.go @@ -13,6 +13,7 @@ import ( v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/tarball" "github.com/google/go-containerregistry/pkg/v1/types" + "github.com/pkg/errors" ) // languageLayerBuilder builds the layer for the given language whuch may @@ -157,6 +158,12 @@ func newDataTarball(source, target string, ignored []string, verbose bool) error return err } } + ok, err := isWithin(link, source) + if err != nil { + return errors.Wrap(err, "error checking if link is outside function root") + } else if !ok { + return fmt.Errorf("links outside Function root disallowed. Link %v references %v", path, link) + } header, err := tar.FileInfoHeader(info, link) if err != nil { @@ -185,6 +192,27 @@ func newDataTarball(source, target string, ignored []string, verbose bool) error }) } +// isWithin returns true if the parent path has the child path within. +// If parent and child are the same target, this is considered within. +func isWithin(parent, child string) (bool, error) { + var err error + // Convert the paths to clean absolute paths. + if parent, err = filepath.Abs(filepath.Clean(parent)); err != nil { + return false, err + } + if child, err = filepath.Abs(filepath.Clean(child)); err != nil { + return false, err + } + // Calculate the relative path between them + relPath, err := filepath.Rel(parent, child) + if err != nil { + return false, err + } + // Child is not within parent if the relative path from parent to child + // requires .. + return !strings.HasPrefix(relPath, ".."), nil +} + // newCertLayer creates the shared data layer in the container file hierarchy and // returns both its descriptor and layer metadata. func newCertsLayer(cfg *buildConfig) (desc v1.Descriptor, layer v1.Layer, err error) { diff --git a/pkg/oci/containerize_test.go b/pkg/oci/containerize_test.go new file mode 100644 index 0000000000..c6298c3e2c --- /dev/null +++ b/pkg/oci/containerize_test.go @@ -0,0 +1,34 @@ +package oci + +import "testing" + +// Test_isWithin ensures that the isWithin method checks +// that various combinations of parent and child paths. +func Test_isWithin(t *testing.T) { + tests := []struct { + parent string + child string + want bool + name string + }{ + {"/", "/b", true, "Base case, a subdir of an absolute path"}, + {"/a", "/ab", false, "Ensure simple substring does not match"}, + {"./", ".", true, "Ensure links are both made absolute"}, + {"/a/b/../c", "/a/c/d/../", true, "Ensure the links are both sanitized"}, + {"/a", "/a/b/../../", false, "Ensure escaping the parent is a mismatch"}, + {"./", "../", false, "Ensure simple relative mismatch"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := isWithin(tt.parent, tt.child) + if err != nil { + t.Fatal(err) + } + if got != tt.want { + t.Log(tt.name) + t.Errorf("isWithin() = %v, want %v", got, tt.want) + } + }) + } + +}