Skip to content

Commit 4fafa69

Browse files
committed
fix: respect umask when unpacking .crate files
Without this, an attacker can leverage globally writable files buried in the `.crate` file. After a user downloaded and unpacked the file, the attacker can then write malicous code to the downloaded sources.
1 parent 789a2fb commit 4fafa69

File tree

4 files changed

+43
-7
lines changed

4 files changed

+43
-7
lines changed

src/cargo/sources/registry/mod.rs

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,9 @@
185185
186186
use std::collections::HashSet;
187187
use std::fs::{File, OpenOptions};
188-
use std::io::{self, Write};
188+
use std::io;
189+
use std::io::Read;
190+
use std::io::Write;
189191
use std::path::{Path, PathBuf};
190192
use std::task::{ready, Poll};
191193

@@ -580,7 +582,9 @@ impl<'cfg> RegistrySource<'cfg> {
580582
let size_limit = max_unpack_size(self.config, tarball.metadata()?.len());
581583
let gz = GzDecoder::new(tarball);
582584
let gz = LimitErrorReader::new(gz, size_limit);
583-
Archive::new(gz)
585+
let mut tar = Archive::new(gz);
586+
set_mask(&mut tar);
587+
tar
584588
};
585589
let prefix = unpack_dir.file_name().unwrap();
586590
let parent = unpack_dir.parent().unwrap();
@@ -908,3 +912,16 @@ fn max_unpack_size(config: &Config, size: u64) -> u64 {
908912

909913
u64::max(max_unpack_size, size * max_compression_ratio as u64)
910914
}
915+
916+
/// Set the current [`umask`] value for the given tarball. No-op on non-Unix
917+
/// platforms.
918+
///
919+
/// On Windows, tar only looks at user permissions and tries to set the "read
920+
/// only" attribute, so no-op as well.
921+
///
922+
/// [`umask`]: https://man7.org/linux/man-pages/man2/umask.2.html
923+
#[allow(unused_variables)]
924+
fn set_mask<R: Read>(tar: &mut Archive<R>) {
925+
#[cfg(unix)]
926+
tar.set_mask(crate::util::get_umask());
927+
}

src/cargo/util/mod.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,24 @@ pub fn try_canonicalize<P: AsRef<Path>>(path: P) -> std::io::Result<PathBuf> {
208208
})
209209
}
210210

211+
/// Get the current [`umask`] value.
212+
///
213+
/// [`umask`]: https://man7.org/linux/man-pages/man2/umask.2.html
214+
#[cfg(unix)]
215+
pub fn get_umask() -> u32 {
216+
use std::sync::OnceLock;
217+
static UMASK: OnceLock<libc::mode_t> = OnceLock::new();
218+
// SAFETY: Syscalls are unsafe. Calling `umask` twice is even unsafer for
219+
// multithreading program, since it doesn't provide a way to retrive the
220+
// value without modifications. We use a static `OnceLock` here to ensure
221+
// it only gets call once during the entire program lifetime.
222+
*UMASK.get_or_init(|| unsafe {
223+
let umask = libc::umask(0o022);
224+
libc::umask(umask);
225+
umask
226+
}) as u32 // it is u16 on macos
227+
}
228+
211229
#[cfg(test)]
212230
mod test {
213231
use super::*;

tests/testsuite/registry.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3452,9 +3452,9 @@ fn set_mask_during_unpacking() {
34523452
.unwrap()
34533453
};
34543454

3455-
// Assuming umask is `0o022`.
3455+
let umask = cargo::util::get_umask();
34563456
let metadata = fs::metadata(src_file_path("src/lib.rs")).unwrap();
3457-
assert_eq!(metadata.mode() & 0o777, 0o666);
3457+
assert_eq!(metadata.mode() & 0o777, 0o666 & !umask);
34583458
let metadata = fs::metadata(src_file_path("example.sh")).unwrap();
3459-
assert_eq!(metadata.mode() & 0o777, 0o777);
3459+
assert_eq!(metadata.mode() & 0o777, 0o777 & !umask);
34603460
}

tests/testsuite/vendor.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1051,10 +1051,11 @@ fn vendor_preserves_permissions() {
10511051

10521052
p.cargo("vendor --respect-source-config").run();
10531053

1054+
let umask = cargo::util::get_umask();
10541055
let metadata = fs::metadata(p.root().join("vendor/bar/src/lib.rs")).unwrap();
1055-
assert_eq!(metadata.mode() & 0o777, 0o644);
1056+
assert_eq!(metadata.mode() & 0o777, 0o644 & !umask);
10561057
let metadata = fs::metadata(p.root().join("vendor/bar/example.sh")).unwrap();
1057-
assert_eq!(metadata.mode() & 0o777, 0o755);
1058+
assert_eq!(metadata.mode() & 0o777, 0o755 & !umask);
10581059
}
10591060

10601061
#[cargo_test]

0 commit comments

Comments
 (0)