agent: add secure_join to prevent softlink escape

This patch fixed the security issue if the container images has
unsafe symlink to the container rootfs and hackers can be exploit
this symlink to hack the guest system. e.g. make directory or files
on guest.

CVE-2015-3629

Fixes: #1219

Signed-off-by: Qingyuan Hou <qingyuan.hou@linux.alibaba.com>

src/agent/protocols/build.rs

@@ -52,4 +52,3 @@ fn replace_text_in_file(file_name: &str, from: &str, to: &str) -> Result<(), std
 
     Ok(())
 }
-

src/agent/rustjail/src/mount.rs

@@ -231,7 +231,7 @@ pub fn init_rootfs(
             if m.r#type == "bind" {
                 for o in &m.options {
                     if let Some(fl) = PROPAGATION.get(o.as_str()) {
-                        let dest = format!("{}{}", &rootfs, &m.destination);
+                        let dest = secure_join(rootfs, &m.destination);
                         mount(None::<&str>, dest.as_str(), None::<&str>, *fl, None::<&str>)?;
                     }
                 }
@@ -668,6 +668,52 @@ fn parse_mount(m: &Mount) -> (MsFlags, String) {
     (flags, data.join(","))
 }
 
+// This function constructs a canonicalized path by combining the `rootfs` and `unsafe_path` elements.
+// The resulting path is guaranteed to be ("below" / "in a directory under") the `rootfs` directory.
+//
+// Parameters:
+//
+// - `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().to_string()));
+            } 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,
     m: &Mount,
@@ -677,7 +723,7 @@ fn mount_from(
     _label: &str,
 ) -> Result<()> {
     let d = String::from(data);
-    let dest = format!("{}{}", rootfs, &m.destination);
+    let dest = secure_join(rootfs, &m.destination);
 
     let src = if m.r#type.as_str() == "bind" {
         let src = fs::canonicalize(m.source.as_str())?;
@@ -961,6 +1007,10 @@ fn readonly_path(path: &str) -> Result<()> {
 mod tests {
     use super::*;
     use crate::skip_if_not_root;
+    use std::fs::create_dir;
+    use std::fs::create_dir_all;
+    use std::fs::remove_dir_all;
+    use std::os::unix::fs;
     use std::os::unix::io::AsRawFd;
     use tempfile::tempdir;
 
@@ -990,7 +1040,7 @@ mod tests {
         );
 
         let rootfs = tempdir().unwrap();
-        let ret = fs::create_dir(rootfs.path().join("dev"));
+        let ret = create_dir(rootfs.path().join("dev"));
         assert!(ret.is_ok(), "Got: {:?}", ret);
 
         spec.root = Some(oci::Root {
@@ -1001,8 +1051,8 @@ mod tests {
         // there is no spec.mounts, but should pass
         let ret = init_rootfs(stdout_fd, &spec, &cpath, &mounts, true);
         assert!(ret.is_ok(), "Should pass. Got: {:?}", ret);
-        let _ = fs::remove_dir_all(rootfs.path().join("dev"));
-        let _ = fs::create_dir(rootfs.path().join("dev"));
+        let _ = remove_dir_all(rootfs.path().join("dev"));
+        let _ = create_dir(rootfs.path().join("dev"));
 
         // Adding bad mount point to spec.mounts
         spec.mounts.push(oci::Mount {
@@ -1020,8 +1070,8 @@ mod tests {
             ret
         );
         spec.mounts.pop();
-        let _ = fs::remove_dir_all(rootfs.path().join("dev"));
-        let _ = fs::create_dir(rootfs.path().join("dev"));
+        let _ = remove_dir_all(rootfs.path().join("dev"));
+        let _ = create_dir(rootfs.path().join("dev"));
 
         // mounting a cgroup
         spec.mounts.push(oci::Mount {
@@ -1034,8 +1084,8 @@ mod tests {
         let ret = init_rootfs(stdout_fd, &spec, &cpath, &mounts, true);
         assert!(ret.is_ok(), "Should pass. Got: {:?}", ret);
         spec.mounts.pop();
-        let _ = fs::remove_dir_all(rootfs.path().join("dev"));
-        let _ = fs::create_dir(rootfs.path().join("dev"));
+        let _ = remove_dir_all(rootfs.path().join("dev"));
+        let _ = create_dir(rootfs.path().join("dev"));
 
         // mounting /dev
         spec.mounts.push(oci::Mount {
@@ -1072,11 +1122,11 @@ mod tests {
         cgroup_mounts.insert("cpu".to_string(), "cpu".to_string());
         cgroup_mounts.insert("memory".to_string(), "memory".to_string());
 
-        let ret = fs::create_dir_all(tempdir.path().join("cgroups"));
+        let ret = create_dir_all(tempdir.path().join("cgroups"));
         assert!(ret.is_ok(), "Should pass. Got {:?}", ret);
-        let ret = fs::create_dir_all(tempdir.path().join("cpu"));
+        let ret = create_dir_all(tempdir.path().join("cpu"));
         assert!(ret.is_ok(), "Should pass. Got {:?}", ret);
-        let ret = fs::create_dir_all(tempdir.path().join("memory"));
+        let ret = create_dir_all(tempdir.path().join("memory"));
         assert!(ret.is_ok(), "Should pass. Got {:?}", ret);
 
         let ret = mount_cgroups(
@@ -1224,4 +1274,89 @@ 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);
+
+            // 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);
+        }
+    }
 }