fix null pointer derefs

The code for creating a Passwd from the fields of the raw syscall result
assumed that the syscall would return valid C strings in all non-error
cases. This is not true, and at least one platform (Android) will
populate the fields with null pointers where they are not supported.

To fix this and prevent the error from happening again, this commit
changes `cstr2string(ptr)` to check for a null pointer, and return an
`Option<String>`, with `None` being the null pointer case. While
arguably it should be the caller's job to check for a null pointer
before calling (since the safety precondition is that the pointer is to
a valid C string), relying on the type checker to force remembering this
edge case is safer in the long run.

Author: jtracey

src/uu/id/src/id.rs

@@ -508,15 +508,15 @@ fn pline(possible_uid: Option<uid_t>) {
     println!(
         "{}:{}:{}:{}:{}:{}:{}:{}:{}:{}",
         pw.name,
-        pw.user_passwd,
+        pw.user_passwd.unwrap_or_default(),
         pw.uid,
         pw.gid,
-        pw.user_access_class,
+        pw.user_access_class.unwrap_or_default(),
         pw.passwd_change_time,
         pw.expiration,
-        pw.user_info,
-        pw.user_dir,
-        pw.user_shell
+        pw.user_info.unwrap_or_default(),
+        pw.user_dir.unwrap_or_default(),
+        pw.user_shell.unwrap_or_default()
     );
 }
 
@@ -527,7 +527,13 @@ fn pline(possible_uid: Option<uid_t>) {
 
     println!(
         "{}:{}:{}:{}:{}:{}:{}",
-        pw.name, pw.user_passwd, pw.uid, pw.gid, pw.user_info, pw.user_dir, pw.user_shell
+        pw.name,
+        pw.user_passwd.unwrap_or_default(),
+        pw.uid,
+        pw.gid,
+        pw.user_info.unwrap_or_default(),
+        pw.user_dir.unwrap_or_default(),
+        pw.user_shell.unwrap_or_default()
     );
 }
 

src/uu/pinky/src/pinky.rs

@@ -245,12 +245,16 @@ fn time_string(ut: &Utmpx) -> String {
     time::strftime("%b %e %H:%M", &ut.login_time()).unwrap() // LC_ALL=C
 }
 
-fn gecos_to_fullname(pw: &Passwd) -> String {
-    let mut gecos = pw.user_info.clone();
+fn gecos_to_fullname(pw: &Passwd) -> Option<String> {
+    let mut gecos = if let Some(gecos) = &pw.user_info {
+        gecos.clone()
+    } else {
+        return None;
+    };
     if let Some(n) = gecos.find(',') {
         gecos.truncate(n);
     }
-    gecos.replace('&', &pw.name.capitalize())
+    Some(gecos.replace('&', &pw.name.capitalize()))
 }
 
 impl Pinky {
@@ -278,8 +282,13 @@ impl Pinky {
         print!("{1:<8.0$}", utmpx::UT_NAMESIZE, ut.user());
 
         if self.include_fullname {
-            if let Ok(pw) = Passwd::locate(ut.user().as_ref()) {
-                print!(" {:<19.19}", gecos_to_fullname(&pw));
+            let fullname = if let Ok(pw) = Passwd::locate(ut.user().as_ref()) {
+                gecos_to_fullname(&pw)
+            } else {
+                None
+            };
+            if let Some(fullname) = fullname {
+                print!(" {:<19.19}", fullname);
             } else {
                 print!(" {:19}", "        ???");
             }
@@ -341,21 +350,24 @@ impl Pinky {
         for u in &self.names {
             print!("Login name: {:<28}In real life: ", u);
             if let Ok(pw) = Passwd::locate(u.as_str()) {
-                println!(" {}", gecos_to_fullname(&pw));
+                let fullname = gecos_to_fullname(&pw).unwrap_or_default();
+                let user_dir = pw.user_dir.unwrap_or_default();
+                let user_shell = pw.user_shell.unwrap_or_default();
+                println!(" {}", fullname);
                 if self.include_home_and_shell {
-                    print!("Directory: {:<29}", pw.user_dir);
-                    println!("Shell:  {}", pw.user_shell);
+                    print!("Directory: {:<29}", user_dir);
+                    println!("Shell:  {}", user_shell);
                 }
                 if self.include_project {
-                    let mut p = PathBuf::from(&pw.user_dir);
+                    let mut p = PathBuf::from(&user_dir);
                     p.push(".project");
                     if let Ok(f) = File::open(p) {
                         print!("Project: ");
                         read_to_console(f);
                     }
                 }
                 if self.include_plan {
-                    let mut p = PathBuf::from(&pw.user_dir);
+                    let mut p = PathBuf::from(&user_dir);
                     p.push(".plan");
                     if let Ok(f) = File::open(p) {
                         println!("Plan:");

src/uucore/src/lib/features/entries.rs

@@ -147,16 +147,16 @@ pub struct Passwd {
     /// AKA passwd.pw_gid
     pub gid: gid_t,
     /// AKA passwd.pw_gecos
-    pub user_info: String,
+    pub user_info: Option<String>,
     /// AKA passwd.pw_shell
-    pub user_shell: String,
+    pub user_shell: Option<String>,
     /// AKA passwd.pw_dir
-    pub user_dir: String,
+    pub user_dir: Option<String>,
     /// AKA passwd.pw_passwd
-    pub user_passwd: String,
+    pub user_passwd: Option<String>,
     /// AKA passwd.pw_class
     #[cfg(any(target_os = "freebsd", target_vendor = "apple"))]
-    pub user_access_class: String,
+    pub user_access_class: Option<String>,
     /// AKA passwd.pw_change
     #[cfg(any(target_os = "freebsd", target_vendor = "apple"))]
     pub passwd_change_time: time_t,
@@ -166,16 +166,21 @@ pub struct Passwd {
 }
 
 /// SAFETY: ptr must point to a valid C string.
-unsafe fn cstr2string(ptr: *const c_char) -> String {
-    CStr::from_ptr(ptr).to_string_lossy().into_owned()
+/// Returns None if ptr is null.
+unsafe fn cstr2string(ptr: *const c_char) -> Option<String> {
+    if !ptr.is_null() {
+        Some(CStr::from_ptr(ptr).to_string_lossy().into_owned())
+    } else {
+        None
+    }
 }
 
 impl Passwd {
     /// SAFETY: All the pointed-to strings must be valid and not change while
     /// the function runs. That means PW_LOCK must be held.
     unsafe fn from_raw(raw: passwd) -> Self {
         Self {
-            name: cstr2string(raw.pw_name),
+            name: cstr2string(raw.pw_name).expect("passwd without name"),
             uid: raw.pw_uid,
             gid: raw.pw_gid,
             user_info: cstr2string(raw.pw_gecos),
@@ -243,7 +248,7 @@ impl Group {
     /// the function runs. That means PW_LOCK must be held.
     unsafe fn from_raw(raw: group) -> Self {
         Self {
-            name: cstr2string(raw.gr_name),
+            name: cstr2string(raw.gr_name).expect("group without name"),
             gid: raw.gr_gid,
         }
     }

tests/by-util/test_pinky.rs

@@ -24,11 +24,14 @@ fn test_capitalize() {
 fn test_long_format() {
     let login = "root";
     let pw: Passwd = Passwd::locate(login).unwrap();
-    let real_name = pw.user_info.replace('&', &pw.name.capitalize());
+    let user_info = pw.user_info.unwrap_or_default();
+    let user_dir = pw.user_dir.unwrap_or_default();
+    let user_shell = pw.user_shell.unwrap_or_default();
+    let real_name = user_info.replace('&', &pw.name.capitalize());
     let ts = TestScenario::new(util_name!());
     ts.ucmd().arg("-l").arg(login).succeeds().stdout_is(format!(
         "Login name: {:<28}In real life:  {}\nDirectory: {:<29}Shell:  {}\n\n",
-        login, real_name, pw.user_dir, pw.user_shell
+        login, real_name, user_dir, user_shell
     ));
 
     ts.ucmd()