Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address GHSA-xr7r-f8xq-vfvv #2663

Merged
merged 2 commits into from
Jan 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions crates/libcontainer/src/process/container_init_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ pub enum InitProcessError {
IoPriorityClass(String),
#[error("call exec sched_setattr error: {0}")]
SchedSetattr(String),
#[error("failed to verify if current working directory is safe")]
InvalidCwd(#[source] nix::Error),
}

type Result<T> = std::result::Result<T, InitProcessError>;
Expand Down Expand Up @@ -548,6 +550,12 @@ pub fn container_init_process(
})?;
}

// Ensure that the current working directory is actually inside the container.
verify_cwd().map_err(|err| {
tracing::error!(?err, "failed to verify cwd");
err
})?;

// add HOME into envs if not exists
let home_in_envs = envs.iter().any(|x| x.starts_with("HOME="));
if !home_in_envs {
Expand Down Expand Up @@ -830,6 +838,28 @@ fn sync_seccomp(
Ok(())
}

// verifyCwd ensures that the current directory is actually inside the mount
// namespace root of the current process.
// Please refer to XXXXXXX(TODO(utam0k): fill in) for more details.
fn verify_cwd() -> Result<()> {
let cwd = unistd::getcwd().map_err(|err| {
if let nix::errno::Errno::ENOENT = err {
// https://man7.org/linux/man-pages/man2/getcwd.2.html
// ENOENT The current working directory has been unlinked.
InitProcessError::InvalidCwd(err)
} else {
InitProcessError::NixOther(err)
}
})?;

if !cwd.is_absolute() {
// This should never happen, but just in case.
return Err(InitProcessError::InvalidCwd(nix::errno::Errno::ENOENT));
}

Ok(())
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
12 changes: 12 additions & 0 deletions crates/libcontainer/src/process/container_main_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::{
fork::{self, CloneCb},
intel_rdt::setup_intel_rdt,
},
syscall::SyscallError,
user_ns::UserNamespaceConfig,
};
use nix::sys::wait::{waitpid, WaitStatus};
Expand All @@ -29,6 +30,8 @@ pub enum ProcessError {
#[error("failed seccomp listener")]
#[cfg(feature = "libseccomp")]
SeccompListener(#[from] crate::process::seccomp_listener::SeccompListenerError),
#[error("failed syscall")]
SyscallOther(#[source] SyscallError),
}

type Result<T> = std::result::Result<T, ProcessError>;
Expand Down Expand Up @@ -69,6 +72,15 @@ pub fn container_main_process(container_args: &ContainerArgs) -> Result<(Pid, bo
})
};

// Before starting the intermediate process, mark all non-stdio open files as O_CLOEXEC
// to ensure we don't leak any file descriptors to the intermediate process.
// Please refer to XXXXXXX(TODO(utam0k): fill in) for more details.
let syscall = container_args.syscall.create_syscall();
syscall.close_range(0).map_err(|err| {
tracing::error!(?err, "failed to cleanup extra fds");
ProcessError::SyscallOther(err)
})?;

let intermediate_pid = fork::container_clone(cb).map_err(|err| {
tracing::error!("failed to fork intermediate process: {}", err);
ProcessError::IntermediateProcessFailed(err)
Expand Down
Loading