Skip to content

archive: contain extraction writes within the destination root #31585

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Jarred-Sumner wants to merge 1 commit into
base: main
Choose a base branch
from
Open

archive: contain extraction writes within the destination root #31585

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 135 additions & 7 deletions src/libarchive/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1401,6 +1401,91 @@ pub fn path_traverses_created_symlink(path: &[u8], created_symlinks: &[Vec<u8>])
false
}

#[cfg(unix)]
pub struct EntryParent {
fd: Option<Fd>,
base_offset: usize,
}

#[cfg(unix)]
impl EntryParent {
pub fn at_root() -> EntryParent {
EntryParent {
fd: None,
base_offset: 0,
}
}

pub fn dir(&self, root: Fd) -> Fd {
self.fd.unwrap_or(root)
}

pub fn base_offset(&self) -> usize {
self.base_offset
}

pub fn entry_name<'a>(&self, full_path: &'a ZStr) -> &'a ZStr {
ZStr::from_slice_with_nul(&full_path.as_bytes_with_nul()[self.base_offset..])
}
}

#[cfg(unix)]
impl Drop for EntryParent {
fn drop(&mut self) {
if let Some(fd) = self.fd.take() {
fd.close();
}
}
}

#[cfg(unix)]
pub fn open_entry_parent(root: Fd, path: &[u8]) -> Option<EntryParent> {
let trimmed_len = path.len() - path.iter().rev().take_while(|&&c| c == b'/').count();
let trimmed = &path[..trimmed_len];
let Some(sep_index) = trimmed.iter().rposition(|&c| c == b'/') else {
return Some(EntryParent::at_root());
};

let mut parent = EntryParent::at_root();
parent.base_offset = sep_index + 1;

let flags =
bun_sys::O::RDONLY | bun_sys::O::DIRECTORY | bun_sys::O::NOFOLLOW | bun_sys::O::CLOEXEC;
let mut component_buf = PathBuffer::default();

for component in trimmed[..sep_index].split(|&c| c == b'/') {
match component {
b"" | b"." => continue,
b".." => return None,
_ => {}
}
if component.len() >= component_buf.len() {
return None;
}
component_buf[..component.len()].copy_from_slice(component);
component_buf[component.len()] = 0;
let component_z = ZStr::from_buf(&component_buf[..], component.len());

let current = parent.dir(root);
let next = match bun_sys::openat(current, component_z, flags, 0) {
Ok(fd) => fd,
Err(err) if err.get_errno() == bun_sys::E::ENOENT => {
let _ = bun_sys::mkdirat(current, component_z, 0o755);
match bun_sys::openat(current, component_z, flags, 0) {
Ok(fd) => fd,
Err(_) => return None,
}
}
Err(_) => return None,
};
if let Some(previous) = parent.fd.replace(next) {
previous.close();
}
}

Some(parent)
}

/// Port of `bun.MakePath.makePath(u16, dir, sub_path)` (bun.zig:2481) — the
/// Windows arm calls `makeOpenPathAccessMaskW`, which component-iterates the
/// wide path and `NtCreateFile`s each prefix with `FILE_OPEN_IF`, walking back
Expand Down Expand Up @@ -1489,6 +1574,7 @@ pub mod archiver {
pub close_handles: bool,
pub log: bool,
pub npm: bool,
pub nofollow_parents: bool,
}

impl Default for ExtractOptions {
Expand All @@ -1498,6 +1584,7 @@ pub mod archiver {
close_handles: true,
log: false,
npm: false,
nofollow_parents: false,
}
}
}
Expand Down Expand Up @@ -1887,6 +1974,30 @@ impl Archiver {
continue;
}

#[cfg(unix)]
let entry_parent = if options.nofollow_parents
&& matches!(
kind,
bun_sys::FileKind::Directory
| bun_sys::FileKind::File
| bun_sys::FileKind::SymLink
) {
match open_entry_parent(dir_fd, path_slice) {
Some(parent) => parent,
None => {
if options.log {
Output::warn(format_args!(
"Skipping entry whose parent could not be resolved inside the extraction directory: {}\n",
bun_core::fmt::fmt_os_path(path_slice, Default::default()),
));
}
continue;
}
}
} else {
EntryParent::at_root()
};

if options.log {
bun_core::prettyln!(
" {}",
Expand Down Expand Up @@ -1927,9 +2038,14 @@ impl Archiver {
let path_z: &ZStr = unsafe {
ZStr::from_raw(path_slice.as_ptr(), path_slice.len())
};
#[cfg(unix)]
let (create_dir, create_name_z) =
(entry_parent.dir(dir_fd), entry_parent.entry_name(path_z));
#[cfg(not(unix))]
let (create_dir, create_name_z) = (dir_fd, path_z);
match bun_sys::mkdirat_z(
dir_fd,
path_z,
create_dir,
create_name_z,
bun_sys::Mode::try_from(mode).expect("int cast"),
) {
Ok(()) => {}
Expand All @@ -1947,7 +2063,8 @@ impl Archiver {
return Err(err.into());
}
let _ = dir.make_path_u8(dirname);
let _ = bun_sys::mkdirat_z(dir_fd, path_z, 0o777);
let _ =
bun_sys::mkdirat_z(create_dir, create_name_z, 0o777);
}
}
}
Expand Down Expand Up @@ -1983,7 +2100,9 @@ impl Archiver {
let path_z: &ZStr = unsafe {
ZStr::from_raw(path_slice.as_ptr(), path_slice.len())
};
match bun_sys::symlinkat(link_target, dir_fd, path_z) {
let (create_dir, create_name_z) =
(entry_parent.dir(dir_fd), entry_parent.entry_name(path_z));
match bun_sys::symlinkat(link_target, create_dir, create_name_z) {
Ok(()) => {}
// PORT NOTE: Zig matched error.EPERM / error.ENOENT (errnoToZigErr maps 1:1).
Err(err) => match err.get_errno() {
Expand All @@ -1993,7 +2112,11 @@ impl Archiver {
return Err(err.into());
}
let _ = dir.make_path_u8(dirname);
bun_sys::symlinkat(link_target, dir_fd, path_z)?;
bun_sys::symlinkat(
link_target,
create_dir,
create_name_z,
)?;
}
_ => return Err(err.into()),
},
Expand Down Expand Up @@ -2068,7 +2191,12 @@ impl Archiver {
let path_z: &ZStr = unsafe {
ZStr::from_raw(path_slice.as_ptr(), path_slice.len())
};
match bun_sys::openat(dir_fd, path_z, flags, mode) {
#[cfg(unix)]
let (create_dir, create_name_z) =
(entry_parent.dir(dir_fd), entry_parent.entry_name(path_z));
#[cfg(not(unix))]
let (create_dir, create_name_z) = (dir_fd, path_z);
match bun_sys::openat(create_dir, create_name_z, flags, mode) {
Ok(fd) => fd,
// PORT NOTE: Zig matched error.AccessDenied / error.FileNotFound.
Err(err) => match err.get_errno() {
Expand All @@ -2080,7 +2208,7 @@ impl Archiver {
return Err(err.into());
}
let _ = dir.make_path_u8(dirname);
bun_sys::openat(dir_fd, path_z, flags, mode)?
bun_sys::openat(create_dir, create_name_z, flags, mode)?
}
_ => return Err(err.into()),
},
Expand Down
51 changes: 33 additions & 18 deletions src/runtime/api/Archive.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,7 @@ impl ExtractContext {
close_handles: true,
log: false,
npm: false,
nofollow_parents: true,
},
) {
Ok(c) => c,
Expand Down Expand Up @@ -1480,9 +1481,34 @@ fn extract_to_disk_filtered(
let filetype = entry_ref.filetype();
let kind = bun_sys::kind_from_mode(filetype);

#[cfg(unix)]
let entry_parent = if matches!(
kind,
bun_sys::FileKind::Directory | bun_sys::FileKind::File | bun_sys::FileKind::SymLink
) {
match libarchive::open_entry_parent(dir_fd, pathname) {
Some(parent) => parent,
None => continue,
}
} else {
libarchive::EntryParent::at_root()
};
#[cfg(unix)]
let entry_dir = entry_parent.dir(dir_fd);
#[cfg(unix)]
let entry_name = &pathname[entry_parent.base_offset()..];
#[cfg(unix)]
let entry_name_z = entry_parent.entry_name(pathname_z);
#[cfg(not(unix))]
let entry_dir = dir_fd;
#[cfg(not(unix))]
let entry_name = pathname;
#[cfg(not(unix))]
let entry_name_z = pathname_z;

match kind {
bun_sys::FileKind::Directory => {
match dir_fd.make_path(pathname) {
match entry_dir.make_path(entry_name) {
// Directory already exists - don't count as extracted
Err(e) if e == bun_core::err!("PathAlreadyExists") => continue,
Err(_) => continue,
Expand All @@ -1501,6 +1527,7 @@ fn extract_to_disk_filtered(
};

// Create parent directories if needed (ignore expected errors)
#[cfg(not(unix))]
if let Some(parent_dir) = bun_core::dirname(pathname) {
match dir_fd.make_path(parent_dir) {
// Expected: directory already exists
Expand All @@ -1515,8 +1542,8 @@ fn extract_to_disk_filtered(

// Create and write the file using bun.sys
let file_fd: Fd = match bun_sys::openat(
dir_fd,
pathname_z,
entry_dir,
entry_name_z,
bun_sys::O::WRONLY | bun_sys::O::CREAT | bun_sys::O::TRUNC,
mode,
) {
Expand Down Expand Up @@ -1565,7 +1592,7 @@ fn extract_to_disk_filtered(
count += 1;
} else {
// Remove partial file on failure
let _ = bun_sys::unlinkat(dir_fd, pathname_z);
let _ = bun_sys::unlinkat(entry_dir, entry_name_z);
}
}
bun_sys::FileKind::SymLink => {
Expand All @@ -1578,20 +1605,8 @@ fn extract_to_disk_filtered(
// On Windows, symlinks are skipped since they require elevated privileges.
#[cfg(unix)]
{
match bun_sys::symlinkat(link_target_z, dir_fd, pathname_z) {
Err(err) => {
if matches!(err.get_errno(), bun_sys::E::EPERM | bun_sys::E::ENOENT) {
if let Some(parent) = bun_core::dirname(pathname) {
let _ = dir_fd.make_path(parent);
}
if bun_sys::symlinkat(link_target_z, dir_fd, pathname_z).is_err() {
continue;
}
} else {
continue;
}
}
Ok(()) => {}
if bun_sys::symlinkat(link_target_z, entry_dir, entry_name_z).is_err() {
continue;
}
count += 1;
}
Expand Down
51 changes: 50 additions & 1 deletion test/js/bun/archive.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, isWindows, tempDir } from "harness";
import { existsSync, readdirSync, rmSync } from "node:fs";
import { existsSync, lstatSync, mkdirSync, readdirSync, rmSync, symlinkSync } from "node:fs";
import { join } from "path";

// Minimal ustar tarball builder (pathnames must be <100 bytes).
Expand Down Expand Up @@ -906,6 +906,55 @@ describe("Bun.Archive", () => {
// The normalized "escaped_dir" may or may not exist inside extractPath
// (depending on whether normalization keeps it), but it must NOT be outside
});

test.skipIf(isWindows)(
"does not write through a pre-existing symlinked directory inside the destination",
async () => {
using dir = tempDir("archive-preexisting-symlink", {});
const extractRoot = join(String(dir), "extract");
const outsideDir = join(String(dir), "outside");
mkdirSync(extractRoot, { recursive: true });
mkdirSync(outsideDir, { recursive: true });
symlinkSync(outsideDir, join(extractRoot, "pivot"), "dir");

const archive = new Bun.Archive({
"pivot/file.txt": "escaped",
"ok.txt": "ok",
});
await archive.extract(extractRoot);

// The entry under the symlinked directory must not be written through the link
expect(existsSync(join(outsideDir, "file.txt"))).toBe(false);
expect(readdirSync(outsideDir)).toEqual([]);
// The pre-existing symlink is left untouched
expect(lstatSync(join(extractRoot, "pivot")).isSymbolicLink()).toBe(true);
// Other entries still extract normally
expect(await Bun.file(join(extractRoot, "ok.txt")).text()).toBe("ok");
},
);

test.skipIf(isWindows)(
"does not write through a pre-existing symlinked directory inside the destination (glob)",
async () => {
using dir = tempDir("archive-preexisting-symlink-glob", {});
const extractRoot = join(String(dir), "extract");
const outsideDir = join(String(dir), "outside");
mkdirSync(extractRoot, { recursive: true });
mkdirSync(outsideDir, { recursive: true });
symlinkSync(outsideDir, join(extractRoot, "pivot"), "dir");

const archive = new Bun.Archive({
"pivot/file.txt": "escaped",
"ok.txt": "ok",
});
await archive.extract(extractRoot, { glob: "**" });

expect(existsSync(join(outsideDir, "file.txt"))).toBe(false);
expect(readdirSync(outsideDir)).toEqual([]);
expect(lstatSync(join(extractRoot, "pivot")).isSymbolicLink()).toBe(true);
expect(await Bun.file(join(extractRoot, "ok.txt")).text()).toBe("ok");
},
);
});

describe("Archive.write()", () => {
Expand Down
Loading