mirror of
https://github.com/opencontainers/runc.git
synced 2026-07-10 21:53:57 +08:00
rootfs: switch to fd-based handling of mountpoint targets
An attacker could race with us during mount configuration in order to trick us into mounting over an unexpected path. This would bypass checkProcMount() and would allow for security profiles to be left unapplied by mounting over /proc/self/attr/... (or even more serious outcomes such as killing the entire system by tricking runc into writing strings to /proc/sysrq-trigger). This is a larger issue with our current mount infrastructure, and the ideal solution would be to rewrite it all to be fd-based (which would also allow us to support the "new" mount API, which also avoids a bunch of other issues with mount(8)). However, such a rewrite is not really workable as a security fix, so this patch is a bit of a compromise approach to fix the issue while also moving us a bit towards that eventual end-goal. The core issue in CVE-2025-52881 is that we currently use the (insecure) SecureJoin to re-resolve mountpoint target paths multiple times during mounting. Rather than generating a string from createMountpoint(), we instead open an *os.File handle to the target mountpoint directly and then operate on that handle. This will make it easier to remove utils.WithProcfd() and rework mountViaFds() in the future. The only real issue we need to work around is that we need to re-open the mount target after doing the mount in order to get a handle to the mountpoint -- pathrs.Reopen() doesn't work in this case (it just re-opens the inode under the mountpoint) so we need to do a naive re-open using the full path. Note that if we used move_mount(2) this wouldn't be a problem because we would have a handle to the mountpoint itself. Note that this is still somewhat of a temporary solution -- ideally mountViaFds would use *os.File directly to let us avoid some other issues with using bare /proc/... paths, as well as also letting us more easily use the new mount API on modern kernels. Fixes: GHSA-cgrx-mc8f-2prm CVE-2025-52881 Co-developed-by: lifubang <lifubang@acmcoder.com> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This commit is contained in:
@@ -0,0 +1,71 @@
|
||||
// SPDX-License-Identifier: Apache-2.0
|
||||
/*
|
||||
* Copyright (C) 2024-2025 Aleksa Sarai <cyphar@cyphar.com>
|
||||
* Copyright (C) 2024-2025 SUSE LLC
|
||||
*
|
||||
* 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 pathrs
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
|
||||
"github.com/cyphar/filepath-securejoin/pathrs-lite"
|
||||
"golang.org/x/sys/unix"
|
||||
|
||||
"github.com/opencontainers/runc/internal/linux"
|
||||
)
|
||||
|
||||
// OpenInRoot opens the given path inside the root with the provided flags. It
|
||||
// is effectively shorthand for [securejoin.OpenInRoot] followed by
|
||||
// [securejoin.Reopen].
|
||||
func OpenInRoot(root, subpath string, flags int) (*os.File, error) {
|
||||
handle, err := pathrs.OpenInRoot(root, subpath)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
defer handle.Close()
|
||||
return pathrs.Reopen(handle, flags)
|
||||
}
|
||||
|
||||
// CreateInRoot creates a new file inside a root (as well as any missing parent
|
||||
// directories) and returns a handle to said file. This effectively has
|
||||
// open(O_CREAT|O_NOFOLLOW) semantics. If you want the creation to use O_EXCL,
|
||||
// include it in the passed flags. The fileMode argument uses unix.* mode bits,
|
||||
// *not* os.FileMode.
|
||||
func CreateInRoot(root, subpath string, flags int, fileMode uint32) (*os.File, error) {
|
||||
dir, filename := filepath.Split(subpath)
|
||||
if filepath.Join("/", filename) == "/" {
|
||||
return nil, fmt.Errorf("create in root subpath %q has bad trailing component %q", subpath, filename)
|
||||
}
|
||||
|
||||
dirFd, err := MkdirAllInRootOpen(root, dir, 0o755)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
defer dirFd.Close()
|
||||
|
||||
// We know that the filename does not have any "/" components, and that
|
||||
// dirFd is inside the root. O_NOFOLLOW will stop us from following
|
||||
// trailing symlinks, so this is safe to do. libpathrs's Root::create_file
|
||||
// works the same way.
|
||||
flags |= unix.O_CREAT | unix.O_NOFOLLOW
|
||||
fd, err := linux.Openat(int(dirFd.Fd()), filename, flags, fileMode)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return os.NewFile(uintptr(fd), root+"/"+subpath), nil
|
||||
}
|
||||
Reference in New Issue
Block a user