From 538b0e779d96880b1152cfbc329229bf9bebe57b Mon Sep 17 00:00:00 2001
From: Otavio Salvador <otavio@ossystems.com.br>
Date: Sat, 31 Jul 2021 10:23:03 -0300
Subject: [PATCH] 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                  |   2 ++
 src/lib.rs                  |  38 +++++++++++++++++++++++++++---------
 tests/fixtures/zip-slip.zip | Bin 0 -> 545 bytes
 tests/integration_test.rs   |  15 ++++++++++++++
 4 files changed, 46 insertions(+), 9 deletions(-)
 create mode 100644 tests/fixtures/zip-slip.zip

diff --git a/CHANGES.md b/CHANGES.md
index 7127913..b581ac1 100644
--- a/CHANGES.md
+++ b/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
 
diff --git a/src/lib.rs b/src/lib.rs
index 3f1caec..d3833f9 100644
--- a/src/lib.rs
+++ b/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,
diff --git a/tests/fixtures/zip-slip.zip b/tests/fixtures/zip-slip.zip
new file mode 100644
index 0000000000000000000000000000000000000000..38b3f499de0163e62ca15ce18350a9d9a477a51b
GIT binary patch
literal 545
zcmWIWW@h1H0D=Au{XYEp{-1?`Y!K#PkYPyA&ri`SsVE5z;bdU8U359h4v0%DxEUB(
zzA-W|u!sQFm1JZVD*#cV0!Xz&eqJh90MJm76a&LlprHwl)s`S02)6*So}T`Ippx7I
z{nWC|9FT|Lj?Pm62|-=W$Rx*%D=;L0E@xl>dYWNLBZ!3v8dgZqpan~SHzSh>Gwx6T
jnE?Vz8bg8PfCLE8QsgiR@MdKLxrhk}K_2A>d6oeH^pk5C

literal 0
HcmV?d00001

diff --git a/tests/integration_test.rs b/tests/integration_test.rs
index 4ed0fa6..4a64d00 100644
--- a/tests/integration_test.rs
+++ b/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!"
+    );
+}