Merge pull request #688 from kzys/fix-join

prepareBindMounts must not modify files outside of the jail

Author: Kazuyoshi Kato

runtime/runc_jailer.go

@@ -25,6 +25,7 @@ import (
 	"syscall"
 	"time"
 
+	"github.com/containerd/continuity/fs"
 	"github.com/containerd/go-runc"
 	"github.com/hashicorp/go-multierror"
 	"github.com/opencontainers/runtime-spec/specs-go"
@@ -144,7 +145,11 @@ func (j *runcJailer) prepareBindMounts(mounts []*proto.FirecrackerDriveMount) er
 		// Only bindMount regular file.
 		// To avoid duplicate files, for block device, we will use system call to create device file for it.
 		if stat.Mode&syscall.S_IFMT == syscall.S_IFREG {
-			err := j.bindMountFileToJail(m.HostPath, filepath.Join(j.RootPath(), m.HostPath))
+			dest, err := fs.RootPath(j.RootPath(), m.HostPath)
+			if err != nil {
+				return err
+			}
+			err = j.bindMountFileToJail(m.HostPath, dest)
 			if err != nil {
 				return err
 			}

runtime/runc_jailer_test.go

@@ -223,3 +223,57 @@ func TestFifoHandler(t *testing.T) {
 		})
 	}
 }
+
+func TestPrepareBindMount(t *testing.T) {
+	// Because of chown(2).
+	internal.RequiresRoot(t)
+
+	t.Run("no mounts", func(t *testing.T) {
+		j := &runcJailer{}
+		err := j.prepareBindMounts([]*proto.FirecrackerDriveMount{})
+		require.NoError(t, err)
+	})
+
+	dir, err := ioutil.TempDir("", t.Name())
+	require.NoError(t, err)
+	defer os.RemoveAll(dir)
+
+	j := &runcJailer{Config: runcJailerConfig{
+		OCIBundlePath: filepath.Join(dir, "bundle"),
+		UID:           1234,
+		GID:           5678,
+	}}
+
+	err = ioutil.WriteFile(dir+"/foobar", []byte("hello"), 0700)
+	require.NoError(t, err)
+
+	testcases := []struct {
+		name     string
+		hostPath string
+	}{
+		{
+			name:     "absolute path",
+			hostPath: dir + "/foobar",
+		},
+		{
+			name:     "use dots to access the original directory",
+			hostPath: "/../../../../../.." + dir + "/foobar",
+		},
+	}
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			err = j.prepareBindMounts([]*proto.FirecrackerDriveMount{{
+				HostPath:       tc.hostPath,
+				FilesystemType: "ext4",
+				VMPath:         "/mnt",
+			}})
+			require.NoError(t, err)
+			stat, err := os.Stat(dir)
+			require.NoError(t, err)
+
+			s := stat.Sys().(*syscall.Stat_t)
+			assert.Equal(t, 0, int(s.Uid), "UID")
+			assert.Equal(t, 0, int(s.Gid), "GID")
+		})
+	}
+}