github/kata-containers
1
0
Fork 1
mirror of https://github.com/kata-containers/kata-containers.git synced 2025-07-05 11:36:56 +00:00
Code Issues Projects Releases Wiki Activity

agent: use safe-path to replace secure_join

Browse Source 
This patch use safe-path library to safely handle filesystem paths.

Signed-off-by: Qingyuan Hou <qingyuan.hou@linux.alibaba.com>
This commit is contained in:
Qingyuan Hou 2025-05-08 07:08:47 +00:00
parent e3e0007bf7
commit c0ceaf661a
3 changed files with 11 additions and 124 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
src/agent/Cargo.lock generated
View File

@ -5487,6 +5487,7 @@ dependencies = [
"regex",
"rlimit",
"runtime-spec",
"safe-path",
"scan_fmt",
"scopeguard",
"serde",

1
src/agent/rustjail/Cargo.toml
View File

@ -21,6 +21,7 @@ protobuf.workspace = true
slog.workspace = true
slog-scope.workspace = true
scan_fmt.workspace = true
safe-path.workspace = true
regex.workspace = true
path-absolutize = "1.2.0"
anyhow = "1.0.32"

133
src/agent/rustjail/src/mount.rs
View File

@ -32,6 +32,7 @@ use crate::sync::write_count;
use std::string::ToString;
use crate::log_child;
use safe_path::scoped_join;
// Info reveals information about a particular mounted filesystem. This
// struct is populated from the content in the /proc/<pid>/mountinfo file.
@ -277,7 +278,10 @@ pub fn init_rootfs(
// first check that we have non-default options required before attempting a
// remount
if mount_typ == "bind" && !pgflags.is_empty() {
let dest = secure_join(rootfs, mount_dest);
let dest = scoped_join(rootfs, mount_dest)?
.to_str()
.ok_or_else(|| anyhow::anyhow!("Failed to convert path to string"))?
.to_string();
mount(
None::<&str>,
dest.as_str(),
@ -763,43 +767,6 @@ fn parse_mount(m: &Mount) -> (MsFlags, MsFlags, String) {
// - `rootfs` is the absolute path to the root of the containers root filesystem directory.
// - `unsafe_path` is path inside a container. It is unsafe since it may try to "escape" from the containers
// rootfs by using one or more "../" path elements or is its a symlink to path.
fn secure_join(rootfs: &str, unsafe_path: &str) -> String {
let mut path = PathBuf::from(format!("{}/", rootfs));
let unsafe_p = Path::new(&unsafe_path);
for it in unsafe_p.iter() {
let it_p = Path::new(&it);
// if it_p leads with "/", path.push(it) will be replace as it, so ignore "/"
if it_p.has_root() {
continue;
};
path.push(it);
if let Ok(v) = path.read_link() {
if v.is_absolute() {
path = PathBuf::from(format!("{}{}", rootfs, v.to_str().unwrap()));
} else {
path.pop();
for it in v.iter() {
path.push(it);
if path.exists() {
path = path.canonicalize().unwrap();
if !path.starts_with(rootfs) {
path = PathBuf::from(rootfs.to_string());
}
}
}
}
}
// skip any ".."
if path.ends_with("..") {
path.pop();
}
}
path.to_str().unwrap().to_string()
}
fn mount_from(
cfd_log: RawFd,
@ -812,7 +779,10 @@ fn mount_from(
let mut d = String::from(data);
let mount_dest = m.destination().display().to_string();
let mount_typ = m.typ().as_ref().unwrap();
let dest = secure_join(rootfs, &mount_dest);
let dest = scoped_join(rootfs, mount_dest)?
.to_str()
.ok_or_else(|| anyhow::anyhow!("Failed to convert path to string"))?
.to_string();
let mount_source = m.source().as_ref().unwrap().display().to_string();
let src = if mount_typ == "bind" {
@ -1617,91 +1587,6 @@ mod tests {
assert!(check_proc_mount(&mount).is_err());
}
#[test]
fn test_secure_join() {
#[derive(Debug)]
struct TestData<'a> {
name: &'a str,
rootfs: &'a str,
unsafe_path: &'a str,
symlink_path: &'a str,
result: &'a str,
}
// create tempory directory to simulate container rootfs with symlink
let rootfs_dir = tempdir().expect("failed to create tmpdir");
let rootfs_path = rootfs_dir.path().to_str().unwrap();
let tests = &[
TestData {
name: "rootfs_not_exist",
rootfs: "/home/rootfs",
unsafe_path: "a/b/c",
symlink_path: "",
result: "/home/rootfs/a/b/c",
},
TestData {
name: "relative_path",
rootfs: "/home/rootfs",
unsafe_path: "../../../a/b/c",
symlink_path: "",
result: "/home/rootfs/a/b/c",
},
TestData {
name: "skip any ..",
rootfs: "/home/rootfs",
unsafe_path: "../../../a/../../b/../../c",
symlink_path: "",
result: "/home/rootfs/a/b/c",
},
TestData {
name: "rootfs is null",
rootfs: "",
unsafe_path: "",
symlink_path: "",
result: "/",
},
TestData {
name: "relative softlink beyond container rootfs",
rootfs: rootfs_path,
unsafe_path: "1",
symlink_path: "../../../",
result: rootfs_path,
},
TestData {
name: "abs softlink points to the non-exist directory",
rootfs: rootfs_path,
unsafe_path: "2",
symlink_path: "/dddd",
result: &format!("{}/dddd", rootfs_path).as_str().to_owned(),
},
TestData {
name: "abs softlink points to the root",
rootfs: rootfs_path,
unsafe_path: "3",
symlink_path: "/",
result: &format!("{}/", rootfs_path).as_str().to_owned(),
},
];
for (i, t) in tests.iter().enumerate() {
// Create a string containing details of the test
let msg = format!("test[{}]: {:?}", i, t.name);
// if is_symlink, then should be prepare the softlink environment
if t.symlink_path != "" {
fs::symlink(t.symlink_path, format!("{}/{}", t.rootfs, t.unsafe_path)).unwrap();
}
let result = secure_join(t.rootfs, t.unsafe_path);
// Update the test details string with the results of the call
let msg = format!("{}, result: {:?}", msg, result);
// Perform the checks
assert!(result == t.result, "{}", msg);
}
}
#[test]
fn test_parse_mount_table() {
#[derive(Debug)]