Fix absolute paths extracting outside of destination directory by str…

…ipping leading '/'

Previously, an absolute entry path would be joined with the destination, but in Rust joining an absolute path onto another one will just return the absolute path which results in files being extracted outside of the destination directory. This commit changes the behaviour to strip the leading '/' from an absolute path, matching behaviour from bsdtar and gnu tar which will extract the files into the destination directory.

CHANGES.md

@@ -7,8 +7,10 @@
 * ci: windows: Use pre-installed vcpkg and fix build [#81]
 * Raise MSRV to 1.49.0
 * Upgrade tokio-util to 0.7.0
+* Fix absolute paths being extracted outside of destination directory [#83]
 
 [#81]: https://github.com/OSSystems/compress-tools-rs/issues/81
+[#83]: https://github.com/OSSystems/compress-tools-rs/issues/83
 
 ## [0.12.2] - 2021-09-23
 

src/lib.rs

@@ -215,13 +215,11 @@ where
                     ffi::ARCHIVE_EOF => return Ok(()),
                     ffi::ARCHIVE_OK => {
                         let target_path = CString::new(
-                            sanetize_destination_path(
-                                &dest.join(
-                                    CStr::from_ptr(ffi::archive_entry_pathname(entry))
-                                        .to_string_lossy()
-                                        .into_owned(),
-                                ),
-                            )?
+                            dest.join(sanitize_destination_path(Path::new(
+                                &CStr::from_ptr(ffi::archive_entry_pathname(entry))
+                                    .to_string_lossy()
+                                    .into_owned(),
+                            ))?)
                             .to_str()
                             .unwrap(),
                         )
@@ -232,9 +230,9 @@ where
                         let link_name = ffi::archive_entry_hardlink(entry);
                         if !link_name.is_null() {
                             let target_path = CString::new(
-                                sanetize_destination_path(&dest.join(
-                                    CStr::from_ptr(link_name).to_string_lossy().into_owned(),
-                                ))?
+                                dest.join(sanitize_destination_path(Path::new(
+                                    &CStr::from_ptr(link_name).to_string_lossy().into_owned(),
+                                ))?)
                                 .to_str()
                                 .unwrap(),
                             )
@@ -453,10 +451,16 @@ where
 }
 
 // This ensures we're not affected by the zip-slip vulnerability. In summary, it
-// uses relative destination paths to unpack files in unexpected places.
+// uses relative destination paths to unpack files in unexpected places. This also
+// handles absolute paths, where the leading '/' will be stripped, matching behaviour
+// from gnu tar and bsdtar.
 //
 // More details can be found at: http://snyk.io/research/zip-slip-vulnerability
-fn sanetize_destination_path(dest: &Path) -> Result<&Path> {
+fn sanitize_destination_path(mut dest: &Path) -> Result<&Path> {
+    if dest.starts_with("/") {
+        dest = dest.strip_prefix("/").unwrap_or(dest);
+    }
+
     dest.components()
         .find(|c| c == &Component::ParentDir)
         .map_or(Ok(dest), |_| {

tests/integration_test.rs

@@ -3,6 +3,7 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
 use compress_tools::*;
+use std::path::Path;
 
 #[test]
 fn get_compressed_file_content() {
@@ -560,3 +561,25 @@ fn uncompress_archive_zip_slip_vulnerability() {
         "SECURITY ERROR: evil.txt has been uncompressed in /tmp!"
     );
 }
+
+#[test]
+fn uncompress_archive_absolute_path() {
+    let temp_dir = tempfile::TempDir::new().expect("Failed to create the tmp directory");
+    let dest = temp_dir.path();
+
+    let correct_dest = dest.join("test.txt");
+    let incorrect_dest = Path::new("/test.txt");
+
+    assert!(
+        uncompress_archive(
+            &mut std::fs::File::open("tests/fixtures/absolute-path.tar").unwrap(),
+            dest,
+            Ownership::Ignore,
+        )
+        .is_ok(),
+        "SECURITY ERROR: test.txt has been uncompressed in /!"
+    );
+
+    assert!(correct_dest.exists());
+    assert!(!Path::new(incorrect_dest).exists());
+}