Merge pull request from GHSA-fmvj-vqp5-qqh9

* Sanitize permissions

* Forbid creating directories under ledger/rocksdb/

* hardened_unpack: Disallow dirs under rocksdb/ in genesis

* hardened_unpack: expand valid genesis entry test coverage

* hardened_unpack: rework old-style bsd directory entry rejection

Co-authored-by: Ivan Mironov <mironov.ivan@gmail.com>

runtime/src/hardened_unpack.rs

@@ -113,15 +113,22 @@ where
             Normal(c) => c.to_str(),
             _ => None, // Prefix (for Windows) and RootDir are forbidden
         });
-        if parts.clone().any(|p| p.is_none()) {
+
+        // Reject old-style BSD directory entries that aren't explicitly tagged as directories
+        let legacy_dir_entry =
+            entry.header().as_ustar().is_none() && entry.path_bytes().ends_with(b"/");
+        let kind = entry.header().entry_type();
+        let reject_legacy_dir_entry = legacy_dir_entry && (kind != Directory);
+
+        if parts.clone().any(|p| p.is_none()) || reject_legacy_dir_entry {
             return Err(UnpackError::Archive(format!(
                 "invalid path found: {:?}",
                 path_str
             )));
         }
 
         let parts: Vec<_> = parts.map(|p| p.unwrap()).collect();
-        let unpack_dir = match entry_checker(parts.as_slice(), entry.header().entry_type()) {
+        let unpack_dir = match entry_checker(parts.as_slice(), kind) {
             None => {
                 return Err(UnpackError::Archive(format!(
                     "extra entry found: {:?} {:?}",
@@ -147,6 +154,14 @@ where
         // unpack_in does its own sanitization
         // ref: https://docs.rs/tar/*/tar/struct.Entry.html#method.unpack_in
         check_unpack_result(entry.unpack_in(unpack_dir)?, path_str)?;
+
+        // Sanitize permissions.
+        let mode = match entry.header().entry_type() {
+            GNUSparse | Regular => 0o644,
+            _ => 0o755,
+        };
+        set_perms(&unpack_dir.join(entry.path()?), mode)?;
+
         total_entries += 1;
         let now = Instant::now();
         if now.duration_since(last_log_update).as_secs() >= 10 {
@@ -156,7 +171,22 @@ where
     }
     info!("unpacked {} entries total", total_entries);
 
-    Ok(())
+    return Ok(());
+
+    #[cfg(unix)]
+    fn set_perms(dst: &Path, mode: u32) -> std::io::Result<()> {
+        use std::os::unix::fs::PermissionsExt;
+
+        let perm = fs::Permissions::from_mode(mode as _);
+        fs::set_permissions(dst, perm)
+    }
+
+    #[cfg(windows)]
+    fn set_perms(dst: &Path, _mode: u32) -> std::io::Result<()> {
+        let mut perm = fs::metadata(dst)?.permissions();
+        perm.set_readonly(false);
+        fs::set_permissions(dst, perm)
+    }
 }
 
 // Map from AppendVec file name to unpacked file system location
@@ -321,8 +351,8 @@ fn is_valid_genesis_archive_entry(parts: &[&str], kind: tar::EntryType) -> bool
         (["genesis.bin"], GNUSparse) => true,
         (["genesis.bin"], Regular) => true,
         (["rocksdb"], Directory) => true,
-        (["rocksdb", ..], GNUSparse) => true,
-        (["rocksdb", ..], Regular) => true,
+        (["rocksdb", _], GNUSparse) => true,
+        (["rocksdb", _], Regular) => true,
         _ => false,
     }
 }
@@ -430,6 +460,10 @@ mod tests {
             &["genesis.bin"],
             tar::EntryType::Regular
         ));
+        assert!(is_valid_genesis_archive_entry(
+            &["genesis.bin"],
+            tar::EntryType::GNUSparse,
+        ));
         assert!(is_valid_genesis_archive_entry(
             &["rocksdb"],
             tar::EntryType::Directory
@@ -439,14 +473,42 @@ mod tests {
             tar::EntryType::Regular
         ));
         assert!(is_valid_genesis_archive_entry(
-            &["rocksdb", "foo", "bar"],
-            tar::EntryType::Regular
+            &["rocksdb", "foo"],
+            tar::EntryType::GNUSparse,
         ));
 
         assert!(!is_valid_genesis_archive_entry(
             &["aaaa"],
             tar::EntryType::Regular
         ));
+        assert!(!is_valid_genesis_archive_entry(
+            &["aaaa"],
+            tar::EntryType::GNUSparse,
+        ));
+        assert!(!is_valid_genesis_archive_entry(
+            &["rocksdb"],
+            tar::EntryType::Regular
+        ));
+        assert!(!is_valid_genesis_archive_entry(
+            &["rocksdb"],
+            tar::EntryType::GNUSparse,
+        ));
+        assert!(!is_valid_genesis_archive_entry(
+            &["rocksdb", "foo"],
+            tar::EntryType::Directory,
+        ));
+        assert!(!is_valid_genesis_archive_entry(
+            &["rocksdb", "foo", "bar"],
+            tar::EntryType::Directory,
+        ));
+        assert!(!is_valid_genesis_archive_entry(
+            &["rocksdb", "foo", "bar"],
+            tar::EntryType::Regular
+        ));
+        assert!(!is_valid_genesis_archive_entry(
+            &["rocksdb", "foo", "bar"],
+            tar::EntryType::GNUSparse
+        ));
     }
 
     fn with_finalize_and_unpack<C>(archive: tar::Builder<Vec<u8>>, checker: C) -> Result<()>
@@ -458,7 +520,11 @@ mod tests {
         let mut archive: Archive<std::io::BufReader<&[u8]>> = Archive::new(reader);
         let temp_dir = tempfile::TempDir::new().unwrap();
 
-        checker(&mut archive, &temp_dir.into_path())
+        checker(&mut archive, temp_dir.path())?;
+        // Check that there is no bad permissions preventing deletion.
+        let result = temp_dir.close();
+        assert_matches!(result, Ok(()));
+        Ok(())
     }
 
     fn finalize_and_unpack_snapshot(archive: tar::Builder<Vec<u8>>) -> Result<()> {
@@ -505,6 +571,65 @@ mod tests {
         assert_matches!(result, Ok(()));
     }
 
+    #[test]
+    fn test_archive_unpack_genesis_bad_perms() {
+        let mut archive = Builder::new(Vec::new());
+
+        let mut header = Header::new_gnu();
+        header.set_path("rocksdb").unwrap();
+        header.set_entry_type(Directory);
+        header.set_size(0);
+        header.set_cksum();
+        let data: &[u8] = &[];
+        archive.append(&header, data).unwrap();
+
+        let mut header = Header::new_gnu();
+        header.set_path("rocksdb/test").unwrap();
+        header.set_size(4);
+        header.set_cksum();
+        let data: &[u8] = &[1, 2, 3, 4];
+        archive.append(&header, data).unwrap();
+
+        // Removing all permissions makes it harder to delete this directory
+        // or work with files inside it.
+        let mut header = Header::new_gnu();
+        header.set_path("rocksdb").unwrap();
+        header.set_entry_type(Directory);
+        header.set_mode(0o000);
+        header.set_size(0);
+        header.set_cksum();
+        let data: &[u8] = &[];
+        archive.append(&header, data).unwrap();
+
+        let result = finalize_and_unpack_genesis(archive);
+        assert_matches!(result, Ok(()));
+    }
+
+    #[test]
+    fn test_archive_unpack_genesis_bad_rocksdb_subdir() {
+        let mut archive = Builder::new(Vec::new());
+
+        let mut header = Header::new_gnu();
+        header.set_path("rocksdb").unwrap();
+        header.set_entry_type(Directory);
+        header.set_size(0);
+        header.set_cksum();
+        let data: &[u8] = &[];
+        archive.append(&header, data).unwrap();
+
+        // tar-rs treats following entry as a Directory to support old tar formats.
+        let mut header = Header::new_gnu();
+        header.set_path("rocksdb/test/").unwrap();
+        header.set_entry_type(Regular);
+        header.set_size(0);
+        header.set_cksum();
+        let data: &[u8] = &[];
+        archive.append(&header, data).unwrap();
+
+        let result = finalize_and_unpack_genesis(archive);
+        assert_matches!(result, Err(UnpackError::Archive(ref message)) if message == "invalid path found: \"rocksdb/test/\"");
+    }
+
     #[test]
     fn test_archive_unpack_snapshot_invalid_path() {
         let mut header = Header::new_gnu();