diff --git a/src/common/resolve.rs b/src/common/resolve.rs index 15f5d0e65..6ff651c4f 100644 --- a/src/common/resolve.rs +++ b/src/common/resolve.rs @@ -112,7 +112,7 @@ pub(crate) fn resolve_target_user_and_group( // when -u is specified but -g is not specified, default -g to the primary group of the specified user (Some(_), None) => { if let Some(user) = &target_user { - target_group = Group::from_gid(user.gid)?; + target_group = Some(user.primary_group()?); } } // when no -u or -g is specified, default to root:root @@ -279,7 +279,7 @@ mod tests { // fallback to root let (user, group) = resolve_target_user_and_group(&None, &None, ¤t_user).unwrap(); assert_eq!(user.name, "root"); - assert_eq!(group.name, ROOT_GROUP_NAME); + assert_eq!(group.name.unwrap(), ROOT_GROUP_NAME); // unknown user let result = @@ -296,7 +296,7 @@ mod tests { resolve_target_user_and_group(&None, &Some(ROOT_GROUP_NAME.into()), ¤t_user) .unwrap(); assert_eq!(user.name, current_user.name); - assert_eq!(group.name, ROOT_GROUP_NAME); + assert_eq!(group.name.unwrap(), ROOT_GROUP_NAME); // fallback to current users group when no group specified let (user, group) = diff --git a/src/su/context.rs b/src/su/context.rs index d95b376ef..dd15accf3 100644 --- a/src/su/context.rs +++ b/src/su/context.rs @@ -99,8 +99,7 @@ impl SuContext { } // resolve target group - let mut group = - Group::from_gid(user.gid)?.ok_or_else(|| Error::GroupNotFound(user.gid.to_string()))?; + let mut group = user.primary_group()?; if !options.supp_group.is_empty() || !options.group.is_empty() { user.groups.clear(); diff --git a/src/sudo/env/tests.rs b/src/sudo/env/tests.rs index 74861342b..3d5265d74 100644 --- a/src/sudo/env/tests.rs +++ b/src/sudo/env/tests.rs @@ -96,7 +96,7 @@ fn create_test_context(sudo_options: &SudoRunOptions) -> Context { let current_group = Group { gid: GroupId::new(1000), - name: "test".to_string(), + name: Some("test".to_string()), }; let root_user = User { @@ -112,7 +112,7 @@ fn create_test_context(sudo_options: &SudoRunOptions) -> Context { let root_group = Group { gid: GroupId::new(0), - name: "root".to_string(), + name: Some("root".to_string()), }; Context { diff --git a/src/system/interface.rs b/src/system/interface.rs index fc4a7ff34..2f8f28f67 100644 --- a/src/system/interface.rs +++ b/src/system/interface.rs @@ -160,7 +160,7 @@ impl UnixGroup for super::Group { self.gid } fn try_as_name(&self) -> Option<&str> { - Some(&self.name) + self.name.as_deref() } } diff --git a/src/system/mod.rs b/src/system/mod.rs index 8af103c0c..7ca1197fe 100644 --- a/src/system/mod.rs +++ b/src/system/mod.rs @@ -479,6 +479,12 @@ impl User { Self::from_uid(Self::real_uid()) } + pub fn primary_group(&self) -> std::io::Result { + // Use from_gid_unchecked here to ensure that we can still resolve when + // the /etc/group entry for the primary group is missing. + Group::from_gid_unchecked(self.gid) + } + pub fn from_name(name_c: &CStr) -> Result, Error> { let max_pw_size = sysconf(libc::_SC_GETPW_R_SIZE_MAX).unwrap_or(16_384); let mut buf = vec![0; max_pw_size as usize]; @@ -511,7 +517,7 @@ impl User { #[cfg_attr(test, derive(PartialEq))] pub struct Group { pub gid: GroupId, - pub name: String, + pub name: Option, } impl Group { @@ -525,11 +531,12 @@ impl Group { let name = unsafe { string_from_ptr(grp.gr_name) }; Group { gid: GroupId::new(grp.gr_gid), - name, + name: Some(name), } } - pub fn from_gid(gid: GroupId) -> std::io::Result> { + /// Lookup group for gid without returning an error when a /etc/group entry is missing. + fn from_gid_unchecked(gid: GroupId) -> std::io::Result { let max_gr_size = sysconf(libc::_SC_GETGR_R_SIZE_MAX).unwrap_or(16_384); let mut buf = vec![0; max_gr_size as usize]; let mut grp = MaybeUninit::uninit(); @@ -545,13 +552,23 @@ impl Group { ) })?; if grp_ptr.is_null() { - Ok(None) + Ok(Group { gid, name: None }) } else { // SAFETY: grp_ptr was not null, and getgrgid_r succeeded, so we have assurances that // the `grp` structure was written to by getgrgid_r let grp = unsafe { grp.assume_init() }; // SAFETY: `pwd` was obtained by a call to getgrXXX_r, as required. - Ok(Some(unsafe { Group::from_libc(&grp) })) + Ok(unsafe { Group::from_libc(&grp) }) + } + } + + pub fn from_gid(gid: GroupId) -> std::io::Result> { + let group = Self::from_gid_unchecked(gid)?; + if group.name.is_none() { + // No entry in /etc/group + Ok(None) + } else { + Ok(Some(group)) } } @@ -947,7 +964,7 @@ mod tests { for &(id, name) in fixed_groups { let root = Group::from_gid(id).unwrap().unwrap(); assert_eq!(root.gid, id); - assert_eq!(root.name, name); + assert_eq!(root.name.unwrap(), name); } } @@ -978,7 +995,7 @@ mod tests { } }, Group { - name: name.to_string(), + name: Some(name.to_string()), gid: GroupId::new(gid), } ) diff --git a/test-framework/sudo-compliance-tests/src/sudo/misc.rs b/test-framework/sudo-compliance-tests/src/sudo/misc.rs index d23b33bf9..133f6ef1f 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/misc.rs +++ b/test-framework/sudo-compliance-tests/src/sudo/misc.rs @@ -217,13 +217,33 @@ fn does_not_panic_on_io_errors_cli_error() -> Result<()> { } #[test] -#[ignore = "gh771"] fn long_username() -> Result<()> { // `useradd` limits usernames to 32 characters // directly write to `/etc/passwd` to work around this limitation let username = "a".repeat(33); let env = Env(SUDOERS_ALL_ALL_NOPASSWD).build()?; + Command::new("sh") + .arg("-c") + .arg(format!( + "echo {username}:x:1000:1000::/tmp:/bin/sh >> /etc/passwd && echo {username}:x:1000: >> /etc/group" + )) + .output(&env)? + .assert_success()?; + + Command::new("sudo") + .arg("-u") + .arg(username) + .arg("true") + .output(&env)? + .assert_success() +} + +#[test] +fn missing_primary_group() -> Result<()> { + let username = "user"; + let env = Env(SUDOERS_ALL_ALL_NOPASSWD).build()?; + Command::new("sh") .arg("-c") .arg(format!(