security: Fix zip-slip vulnerability

It uses relative destination paths to unpack files in unexpected places.

More details can be found at: http://snyk.io/research/zip-slip-vulnerability

Fixes: #63
Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>

CHANGES.md

@@ -5,10 +5,12 @@
 ## [Unreleased] - ReleaseDate
 
 * Use "lossy" strings for invalid filenames. [#59]
+* Fix zip-slip vulnerability. [#63]
 * Fix memory leak when dropping locale guard. [#64]
 * Add `ArchiveIterator` type. [#65]
 
 [#59]: https://github.com/OSSystems/compress-tools-rs/issues/59
+[#63]: https://github.com/OSSystems/compress-tools-rs/issues/63
 [#64]: https://github.com/OSSystems/compress-tools-rs/issues/64
 [#65]: https://github.com/OSSystems/compress-tools-rs/issues/65
 

src/lib.rs

@@ -67,7 +67,7 @@ use std::{
     ffi::{CStr, CString},
     io::{self, Read, Write},
     os::raw::{c_int, c_void},
-    path::Path,
+    path::{Component, Path},
     slice,
 };
 
@@ -215,11 +215,13 @@ where
                     ffi::ARCHIVE_EOF => return Ok(()),
                     ffi::ARCHIVE_OK => {
                         let target_path = CString::new(
-                            dest.join(
-                                CStr::from_ptr(ffi::archive_entry_pathname(entry))
-                                    .to_string_lossy()
-                                    .into_owned(),
-                            )
+                            sanetize_destination_path(
+                                &dest.join(
+                                    CStr::from_ptr(ffi::archive_entry_pathname(entry))
+                                        .to_string_lossy()
+                                        .into_owned(),
+                                ),
+                            )?
                             .to_str()
                             .unwrap(),
                         )
@@ -230,9 +232,11 @@ where
                         let link_name = ffi::archive_entry_hardlink(entry);
                         if !link_name.is_null() {
                             let target_path = CString::new(
-                                dest.join(CStr::from_ptr(link_name).to_string_lossy().into_owned())
-                                    .to_str()
-                                    .unwrap(),
+                                sanetize_destination_path(&dest.join(
+                                    CStr::from_ptr(link_name).to_string_lossy().into_owned(),
+                                ))?
+                                .to_str()
+                                .unwrap(),
                             )
                             .unwrap();
 
@@ -448,6 +452,22 @@ 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.
+//
+// More details can be found at: http://snyk.io/research/zip-slip-vulnerability
+fn sanetize_destination_path(dest: &Path) -> Result<&Path> {
+    dest.components()
+        .find(|c| c == &Component::ParentDir)
+        .map_or(Ok(dest), |_| {
+            Err(std::io::Error::new(
+                std::io::ErrorKind::InvalidData,
+                "cannot use relative destination directory",
+            )
+            .into())
+        })
+}
+
 fn libarchive_copy_data(
     archive_reader: *mut ffi::archive,
     archive_writer: *mut ffi::archive,

tests/integration_test.rs

@@ -545,3 +545,18 @@ fn iterate_truncated_archive() {
 
     panic!("Did not find expected error");
 }
+
+#[test]
+fn uncompress_archive_zip_slip_vulnerability() {
+    assert!(
+        uncompress_archive(
+            &mut std::fs::File::open("tests/fixtures/zip-slip.zip").unwrap(),
+            tempfile::TempDir::new()
+                .expect("Failed to create the tmp directory")
+                .path(),
+            Ownership::Ignore,
+        )
+        .is_err(),
+        "SECURITY ERROR: evil.txt has been uncompressed in /tmp!"
+    );
+}