imagebuildah: try to rein in use of transport names in image specs

Try to limit which image transports we accept in stages, and scope the
ones that use path names to the context directory.  At some point
anything that isn't an image ID or pullable spec should start being
rejected.

Signed-off-by: Nalin Dahyabhai <[email protected]>

Author: nalind

imagebuildah/stage_executor.go

@@ -21,19 +21,28 @@ import (
 	"github.com/containers/buildah/internal"
 	"github.com/containers/buildah/internal/tmpdir"
 	internalUtil "github.com/containers/buildah/internal/util"
+	"github.com/containers/buildah/internal/volumes"
 	"github.com/containers/buildah/pkg/parse"
 	"github.com/containers/buildah/pkg/rusage"
 	"github.com/containers/buildah/util"
 	config "github.com/containers/common/pkg/config"
 	cp "github.com/containers/image/v5/copy"
+	directoryTransport "github.com/containers/image/v5/directory"
+	dockerTransport "github.com/containers/image/v5/docker"
 	imagedocker "github.com/containers/image/v5/docker"
+	dockerArchiveTransport "github.com/containers/image/v5/docker/archive"
 	"github.com/containers/image/v5/docker/reference"
 	"github.com/containers/image/v5/manifest"
+	ociArchiveTransport "github.com/containers/image/v5/oci/archive"
+	ociLayoutTransport "github.com/containers/image/v5/oci/layout"
+	openshiftTransport "github.com/containers/image/v5/openshift"
 	is "github.com/containers/image/v5/storage"
 	"github.com/containers/image/v5/transports"
+	"github.com/containers/image/v5/transports/alltransports"
 	"github.com/containers/image/v5/types"
 	"github.com/containers/storage"
 	"github.com/containers/storage/pkg/chrootarchive"
+	"github.com/containers/storage/pkg/mount"
 	"github.com/containers/storage/pkg/unshare"
 	docker "github.com/fsouza/go-dockerclient"
 	buildkitparser "github.com/moby/buildkit/frontend/dockerfile/parser"
@@ -920,6 +929,58 @@ func (s *StageExecutor) UnrecognizedInstruction(step *imagebuilder.Step) error {
 	return errors.New(err)
 }
 
+// do our best to ensure that image specifiers that include a transport that
+// uses path names are scoped to the build context directory
+func (s *StageExecutor) sanitizeFrom(from, tmpdir string) (string, string, error) {
+	ref, err := alltransports.ParseImageName(from)
+	if err != nil {
+		if _, err = reference.ParseNormalizedNamed(from); err == nil {
+			// this is a normal-looking image-in-a-registry-or-named-in-storage name
+			return from, "", nil
+		}
+		if img, err := s.executor.store.Image(from); img != nil && err == nil {
+			// this is an image ID
+			return from, "", nil
+		}
+		return "", "", fmt.Errorf("parsing image name %q: %w", from, err)
+	}
+	// TODO: drop this switch block and just return an error... someday
+	switch ref.Transport().Name() {
+	case dockerTransport.Transport.Name(), "docker-daemon", openshiftTransport.Transport.Name():
+		return from, "", nil
+	case dockerArchiveTransport.Transport.Name(), ociLayoutTransport.Transport.Name(), ociArchiveTransport.Transport.Name():
+		// these all take the form path[:stuff]
+		transportRef := ref.StringWithinTransport()
+		parts := strings.Split(transportRef, ":")
+		// the current directory is the root directory is the context directory
+		boundRef, err := volumes.BindFromChroot(s.executor.contextDir, parts[0], tmpdir)
+		if err != nil {
+			return "", "", fmt.Errorf("ensuring that %q is in the context directory: %w", ref.StringWithinTransport(), err)
+		}
+		parts[0] = boundRef
+		transportRef = strings.Join(parts, ":")
+		newRef, err := ref.Transport().ParseReference(transportRef)
+		if err != nil {
+			return "", "", fmt.Errorf("parsing %q as an image name using %q: %w", transportRef, ref.Transport().Name(), err)
+		}
+		return transports.ImageName(newRef), boundRef, nil
+	case directoryTransport.Transport.Name():
+		// this takes the form of just a path
+		// the current directory is the root directory is the context directory
+		transportRef := ref.StringWithinTransport()
+		boundRef, err := volumes.BindFromChroot(s.executor.contextDir, ref.StringWithinTransport(), tmpdir)
+		if err != nil {
+			return "", "", fmt.Errorf("ensuring that %q is in the context directory: %w", ref.StringWithinTransport(), err)
+		}
+		newRef, err := ref.Transport().ParseReference(boundRef)
+		if err != nil {
+			return "", "", fmt.Errorf("parsing %q as an image name using %q: %w", transportRef, ref.Transport().Name(), err)
+		}
+		return transports.ImageName(newRef), boundRef, nil
+	}
+	return "", "", fmt.Errorf("unexpected container image transport %q", ref.Transport().Name())
+}
+
 // prepare creates a working container based on the specified image, or if one
 // isn't specified, the first argument passed to the first FROM instruction we
 // can find in the stage's parsed tree.
@@ -936,6 +997,20 @@ func (s *StageExecutor) prepare(ctx context.Context, from string, initializeIBCo
 		}
 		from = base
 	}
+	sanitizedFrom, intermediateMount, err := s.sanitizeFrom(from, tmpdir.GetTempDir())
+	if err != nil {
+		return nil, fmt.Errorf("invalid base image specification %q: %w", from, err)
+	}
+	if intermediateMount != "" {
+		defer func() {
+			if err := mount.Unmount(intermediateMount); err != nil {
+				logrus.Debugf("unmounting bound version of %q: %v", from, err)
+			}
+			if err := os.Remove(intermediateMount); err != nil {
+				logrus.Debugf("removing dummy version of %q: %v", from, err)
+			}
+		}()
+	}
 	displayFrom := from
 	if ib.Platform != "" {
 		displayFrom = "--platform=" + ib.Platform + " " + displayFrom
@@ -974,7 +1049,7 @@ func (s *StageExecutor) prepare(ctx context.Context, from string, initializeIBCo
 
 	builderOptions := buildah.BuilderOptions{
 		Args:                  ib.Args,
-		FromImage:             from,
+		FromImage:             sanitizedFrom,
 		GroupAdd:              s.executor.groupAdd,
 		PullPolicy:            pullPolicy,
 		ContainerSuffix:       s.executor.containerSuffix,

internal/volumes/bind_linux.go

@@ -11,12 +11,12 @@ import (
 	"golang.org/x/sys/unix"
 )
 
-// bindFromChroot opens "path" inside of "root" using a chrooted subprocess
+// BindFromChroot opens "path" inside of "root" using a chrooted subprocess
 // that returns a descriptor, then creates a uniquely-named temporary directory
 // or file under "tmp" and bind-mounts the opened descriptor to it, returning
 // the path of the temporary file or directory.  The caller is responsible for
 // unmounting and removing the temporary.
-func bindFromChroot(root, path, tmp string) (string, error) {
+func BindFromChroot(root, path, tmp string) (string, error) {
 	fd, _, err := open.InChroot(root, "", path, unix.O_DIRECTORY|unix.O_RDONLY, 0)
 	if err != nil {
 		if !errors.Is(err, unix.ENOTDIR) {

internal/volumes/bind_linux_test.go

@@ -21,11 +21,11 @@ func TestBindFromChroot(t *testing.T) {
 	require.NoError(t, os.Mkdir(filepath.Join(rootdir, "subdirectory"), 0o700), "creating bind mount source directory")
 	require.NoError(t, os.WriteFile(filepath.Join(rootdir, "subdirectory", "file"), []byte(contents1), 0o600))
 	require.NoError(t, os.WriteFile(filepath.Join(rootdir, "file"), []byte(contents2), 0o600))
-	subdir, err := bindFromChroot(rootdir, "subdirectory", destdir)
+	subdir, err := BindFromChroot(rootdir, "subdirectory", destdir)
 	require.NoError(t, err, "bind mounting from a directory")
 	bytes1, err := os.ReadFile(filepath.Join(subdir, "file"))
 	require.NoError(t, err, "reading file from bind-mounted directory")
-	subfile, err := bindFromChroot(rootdir, "file", destdir)
+	subfile, err := BindFromChroot(rootdir, "file", destdir)
 	require.NoError(t, err, "bind mounting from a file")
 	bytes2, err := os.ReadFile(subfile)
 	require.NoError(t, err, "reading file from bind mounted file")

internal/volumes/bind_notlinux.go

@@ -4,12 +4,12 @@ package volumes
 
 import "errors"
 
-// bindFromChroot would open "path" inside of "root" using a chrooted
+// BindFromChroot would open "path" inside of "root" using a chrooted
 // subprocess that returns a descriptor, then would create a uniquely-named
 // temporary directory or file under "tmp" and bind-mount the opened descriptor
 // to it, returning the path of the temporary file or directory.  The caller
 // would be responsible for unmounting and removing the temporary.  For now,
 // this just returns an error because it is not implemented for this platform.
-func bindFromChroot(root, path, tmp string) (string, error) {
+func BindFromChroot(root, path, tmp string) (string, error) {
 	return "", errors.New("not available on this system")
 }

internal/volumes/volumes.go

@@ -327,7 +327,7 @@ func GetBindMount(sys *types.SystemContext, args []string, contextDir string, st
 			return newMount, "", "", "", fmt.Errorf("computing pathname of bind subdirectory: %w", err)
 		}
 		if rel != "." && rel != "/" {
-			mnt, err := bindFromChroot(contextDir, rel, tmpDir)
+			mnt, err := BindFromChroot(contextDir, rel, tmpDir)
 			if err != nil {
 				return newMount, "", "", "", fmt.Errorf("sanitizing bind subdirectory %q: %w", newMount.Source, err)
 			}
@@ -655,7 +655,7 @@ func GetCacheMount(sys *types.SystemContext, args []string, store storage.Store,
 			return newMount, "", "", "", nil, fmt.Errorf("computing pathname of cache subdirectory: %w", err)
 		}
 		if rel != "." && rel != "/" {
-			mnt, err := bindFromChroot(thisCacheRoot, rel, tmpDir)
+			mnt, err := BindFromChroot(thisCacheRoot, rel, tmpDir)
 			if err != nil {
 				return newMount, "", "", "", nil, fmt.Errorf("sanitizing cache subdirectory %q: %w", newMount.Source, err)
 			}

tests/bud.bats

@@ -7340,3 +7340,25 @@ EOF
   find ${TEST_SCRATCH_DIR}/buildcontext -ls
   expect_output "" "build should not be able to write to build context"
 }
+
+@test "build-oci-archive-switch" {
+  base=busybox
+  _prefetch $base
+  run_buildah from -q $base
+  run_buildah inspect --format '{{.FromImageID}}' "$output"
+  imageID="$output"
+  mkdir -p ${TEST_SCRATCH_DIR}/buildcontext
+  copy containers-storage:$imageID oci-archive:${TEST_SCRATCH_DIR}/buildcontext/archive1.tar
+  cat > ${TEST_SCRATCH_DIR}/buildcontext/Dockerfile << EOF
+  FROM $base
+  COPY archive1.tar /
+  RUN --mount=type=bind,rw,target=/bc cp /archive1.tar /bc/archive2.tar
+  RUN --mount=type=bind,rw,target=/bc mkdir /bc/archive3
+  RUN --mount=type=bind,rw,target=/bc tar x -C /bc/archive3 -f /bc/archive2.tar
+  FROM oci-archive:archive2.tar
+  RUN --mount=type=bind,from=0,target=/var/empty :
+  FROM oci:./archive3
+  RUN --mount=type=bind,from=0,target=/var/empty --mount=type=bind,from=1,target=/var/empty2 :
+EOF
+  run_buildah build ${TEST_SCRATCH_DIR}/buildcontext
+}