diff --git a/.github/contributors.yaml b/.github/contributors.yaml index 7f7307a3d..e1887e214 100644 --- a/.github/contributors.yaml +++ b/.github/contributors.yaml @@ -128,3 +128,6 @@ users: OdysseasKalaitsidis: name: Odysseas Kalaitsidis email: odysseaskalaitsides@gmail.com + tagadearpit: + name: Arpit Tagade + email: 143588157+tagadearpit@users.noreply.github.com diff --git a/cmd/urunc/delete.go b/cmd/urunc/delete.go index 7ba25ec00..2be67d67b 100644 --- a/cmd/urunc/delete.go +++ b/cmd/urunc/delete.go @@ -17,10 +17,11 @@ package main import ( "context" "errors" + "fmt" "os" - "path/filepath" "runtime" + securejoin "github.com/cyphar/filepath-securejoin" "github.com/sirupsen/logrus" "github.com/urfave/cli/v3" ) @@ -62,11 +63,15 @@ status of "ubuntu01" as "stopped" the following will delete resources held for return ErrEmptyContainerID } rootDir := cmd.String("root") - containerDir := filepath.Join(rootDir, containerID) - e := os.RemoveAll(containerDir) + containerDir, e := securejoin.SecureJoin(rootDir, containerID) + if e != nil { + return fmt.Errorf("failed to resolve container directory: %w", e) + } + e = os.RemoveAll(containerDir) if e != nil { logrus.Errorf("remove %s: %v", containerDir, e) } + if cmd.Bool("force") { return nil } diff --git a/pkg/unikontainers/path_security_test.go b/pkg/unikontainers/path_security_test.go new file mode 100644 index 000000000..77bd289dc --- /dev/null +++ b/pkg/unikontainers/path_security_test.go @@ -0,0 +1,57 @@ +// Copyright (c) 2023-2026, Nubificus LTD +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package unikontainers + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/opencontainers/runtime-spec/specs-go" + "github.com/stretchr/testify/require" +) + +func TestGetRejectsContainerDirSymlink(t *testing.T) { + rootDir := t.TempDir() + outsideDir := t.TempDir() + bundleDir := t.TempDir() + + spec := &specs.Spec{ + Version: "1.0.2", + Linux: &specs.Linux{}, + } + specData, err := json.Marshal(spec) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(bundleDir, configFilename), specData, 0o600)) + + state := &specs.State{ + Version: "1.0.2", + ID: "victim", + Bundle: bundleDir, + Annotations: map[string]string{ + annotType: "linux", + annotHypervisor: "qemu", + }, + } + stateData, err := json.Marshal(state) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(outsideDir, stateFilename), stateData, 0o600)) + + require.NoError(t, os.Symlink(outsideDir, filepath.Join(rootDir, state.ID))) + + _, err = Get(state.ID, rootDir) + require.ErrorIs(t, err, os.ErrNotExist) +} diff --git a/pkg/unikontainers/unikontainers.go b/pkg/unikontainers/unikontainers.go index ad0bf2897..6f6412aac 100644 --- a/pkg/unikontainers/unikontainers.go +++ b/pkg/unikontainers/unikontainers.go @@ -29,6 +29,7 @@ import ( "sync" "syscall" + securejoin "github.com/cyphar/filepath-securejoin" "github.com/urunc-dev/urunc/pkg/network" "github.com/urunc-dev/urunc/pkg/unikontainers/hypervisors" "github.com/urunc-dev/urunc/pkg/unikontainers/types" @@ -93,7 +94,10 @@ func New(bundlePath string, containerID string, rootDir string, cfg *UruncConfig confMap := config.Map() maps.Copy(confMap, cfg.Map()) - containerDir := filepath.Join(rootDir, containerID) + containerDir, err := securejoin.SecureJoin(rootDir, containerID) + if err != nil { + return nil, fmt.Errorf("failed to resolve container directory: %w", err) + } state := &specs.State{ Version: spec.Version, ID: containerID, @@ -114,7 +118,10 @@ func New(bundlePath string, containerID string, rootDir string, cfg *UruncConfig // Get retrieves unikernel data from disk to create a Unikontainer object func Get(containerID string, rootDir string) (*Unikontainer, error) { u := &Unikontainer{} - containerDir := filepath.Join(rootDir, containerID) + containerDir, err := securejoin.SecureJoin(rootDir, containerID) + if err != nil { + return nil, fmt.Errorf("failed to resolve container directory: %w", err) + } stateFilePath := filepath.Join(containerDir, stateFilename) state, err := loadUnikontainerState(stateFilePath) if err != nil {