mirror of
https://github.com/opencontainers/runc.git
synced 2026-07-11 06:03:57 +08:00
d40b3439a9
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>
117 lines
3.4 KiB
Go
117 lines
3.4 KiB
Go
// Package utils provides general helper utilities used in libcontainer.
|
|
package utils
|
|
|
|
import (
|
|
"encoding/json"
|
|
"io"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
|
|
"golang.org/x/sys/unix"
|
|
)
|
|
|
|
const (
|
|
exitSignalOffset = 128
|
|
)
|
|
|
|
// ExitStatus returns the correct exit status for a process based on if it
|
|
// was signaled or exited cleanly
|
|
func ExitStatus(status unix.WaitStatus) int {
|
|
if status.Signaled() {
|
|
return exitSignalOffset + int(status.Signal())
|
|
}
|
|
return status.ExitStatus()
|
|
}
|
|
|
|
// WriteJSON writes the provided struct v to w using standard json marshaling
|
|
// without a trailing newline. This is used instead of json.Encoder because
|
|
// there might be a problem in json decoder in some cases, see:
|
|
// https://github.com/docker/docker/issues/14203#issuecomment-174177790
|
|
func WriteJSON(w io.Writer, v any) error {
|
|
data, err := json.Marshal(v)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
_, err = w.Write(data)
|
|
return err
|
|
}
|
|
|
|
// CleanPath makes a path safe for use with filepath.Join. This is done by not
|
|
// only cleaning the path, but also (if the path is relative) adding a leading
|
|
// '/' and cleaning it (then removing the leading '/'). This ensures that a
|
|
// path resulting from prepending another path will always resolve to lexically
|
|
// be a subdirectory of the prefixed path. This is all done lexically, so paths
|
|
// that include symlinks won't be safe as a result of using CleanPath.
|
|
func CleanPath(path string) string {
|
|
// Deal with empty strings nicely.
|
|
if path == "" {
|
|
return ""
|
|
}
|
|
|
|
// Ensure that all paths are cleaned (especially problematic ones like
|
|
// "/../../../../../" which can cause lots of issues).
|
|
|
|
if filepath.IsAbs(path) {
|
|
return filepath.Clean(path)
|
|
}
|
|
|
|
// If the path isn't absolute, we need to do more processing to fix paths
|
|
// such as "../../../../<etc>/some/path". We also shouldn't convert absolute
|
|
// paths to relative ones.
|
|
path = filepath.Clean(string(os.PathSeparator) + path)
|
|
// This can't fail, as (by definition) all paths are relative to root.
|
|
path, _ = filepath.Rel(string(os.PathSeparator), path)
|
|
|
|
return path
|
|
}
|
|
|
|
// StripRoot returns the passed path, stripping the root path if it was
|
|
// (lexicially) inside it. Note that both passed paths will always be treated
|
|
// as absolute, and the returned path will also always be absolute. In
|
|
// addition, the paths are cleaned before stripping the root.
|
|
func StripRoot(root, path string) string {
|
|
// Make the paths clean and absolute.
|
|
root, path = CleanPath("/"+root), CleanPath("/"+path)
|
|
switch {
|
|
case path == root:
|
|
path = "/"
|
|
case root == "/":
|
|
// do nothing
|
|
default:
|
|
path = strings.TrimPrefix(path, root+"/")
|
|
}
|
|
return CleanPath("/" + path)
|
|
}
|
|
|
|
// SearchLabels searches through a list of key=value pairs for a given key,
|
|
// returning its value, and the binary flag telling whether the key exist.
|
|
func SearchLabels(labels []string, key string) (string, bool) {
|
|
key += "="
|
|
for _, s := range labels {
|
|
if val, ok := strings.CutPrefix(s, key); ok {
|
|
return val, true
|
|
}
|
|
}
|
|
return "", false
|
|
}
|
|
|
|
// Annotations returns the bundle path and user defined annotations from the
|
|
// libcontainer state. We need to remove the bundle because that is a label
|
|
// added by libcontainer.
|
|
func Annotations(labels []string) (bundle string, userAnnotations map[string]string) {
|
|
userAnnotations = make(map[string]string)
|
|
for _, l := range labels {
|
|
name, value, ok := strings.Cut(l, "=")
|
|
if !ok {
|
|
continue
|
|
}
|
|
if name == "bundle" {
|
|
bundle = value
|
|
} else {
|
|
userAnnotations[name] = value
|
|
}
|
|
}
|
|
return bundle, userAnnotations
|
|
}
|