Merge pull request #2917 from kolyshkin/validate-mounts

Validate mounts
opencontainers · Apr 22, 2021 · d061b41 · d061b41
2 parents 76559c6 + 2192670
commit d061b41
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 3 deletions.
diff --git a/libcontainer/configs/validate/validator.go b/libcontainer/configs/validate/validator.go
@@ -39,6 +39,7 @@ func (v *ConfigValidator) Validate(config *configs.Config) error {
 		v.sysctl,
 		v.intelrdt,
 		v.rootlessEUID,
+		v.mounts,
 	}
 	for _, c := range checks {
 		if err := c(config); err != nil {
@@ -246,6 +247,16 @@ func (v *ConfigValidator) cgroups(config *configs.Config) error {
 	return nil
 }
 
+func (v *ConfigValidator) mounts(config *configs.Config) error {
+	for _, m := range config.Mounts {
+		if !filepath.IsAbs(m.Destination) {
+			return fmt.Errorf("invalid mount %+v: mount destination not absolute", m)
+		}
+	}
+
+	return nil
+}
+
 func isHostNetNS(path string) (bool, error) {
 	const currentProcessNetns = "/proc/self/ns/net"
 

diff --git a/libcontainer/configs/validate/validator_test.go b/libcontainer/configs/validate/validator_test.go
@@ -317,3 +317,36 @@ func TestValidateSysctlWithoutNETNamespace(t *testing.T) {
 		t.Error("Expected error to occur but it was nil")
 	}
 }
+
+func TestValidateMounts(t *testing.T) {
+	testCases := []struct {
+		isErr bool
+		dest  string
+	}{
+		{isErr: true, dest: "not/an/abs/path"},
+		{isErr: true, dest: "./rel/path"},
+		{isErr: true, dest: "./rel/path"},
+		{isErr: true, dest: "../../path"},
+		{isErr: false, dest: "/abs/path"},
+		{isErr: false, dest: "/abs/but/../unclean"},
+	}
+
+	validator := validate.New()
+
+	for _, tc := range testCases {
+		config := &configs.Config{
+			Rootfs: "/var",
+			Mounts: []*configs.Mount{
+				{Destination: tc.dest},
+			},
+		}
+
+		err := validator.Validate(config)
+		if tc.isErr && err == nil {
+			t.Errorf("mount dest: %s, expected error, got nil", tc.dest)
+		}
+		if !tc.isErr && err != nil {
+			t.Errorf("mount dest: %s, expected nil, got error %v", tc.dest, err)
+		}
+	}
+}
diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go
@@ -238,7 +238,11 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) {
 	}
 
 	for _, m := range spec.Mounts {
-		config.Mounts = append(config.Mounts, createLibcontainerMount(cwd, m))
+		cm, err := createLibcontainerMount(cwd, m)
+		if err != nil {
+			return nil, fmt.Errorf("invalid mount %+v: %w", m, err)
+		}
+		config.Mounts = append(config.Mounts, cm)
 	}
 
 	defaultDevs, err := createDevices(spec, config)
@@ -327,7 +331,10 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) {
 	return config, nil
 }
 
-func createLibcontainerMount(cwd string, m specs.Mount) *configs.Mount {
+func createLibcontainerMount(cwd string, m specs.Mount) (*configs.Mount, error) {
+	if !filepath.IsAbs(m.Destination) {
+		return nil, fmt.Errorf("mount destination %s not absolute", m.Destination)
+	}
 	flags, pgflags, data, ext := parseMountOptions(m.Options)
 	source := m.Source
 	device := m.Type
@@ -348,7 +355,7 @@ func createLibcontainerMount(cwd string, m specs.Mount) *configs.Mount {
 		Flags:            flags,
 		PropagationFlags: pgflags,
 		Extensions:       ext,
-	}
+	}, nil
 }
 
 // systemd property name check: latin letters only, at least 3 of them