From 5b08b6b783bcdb0a3bfdfdcfc2e19b3b1f1eb8bd Mon Sep 17 00:00:00 2001 From: Mitchell Paulus Date: Tue, 7 Jul 2026 09:46:43 -0500 Subject: [PATCH 1/5] Add tar builtins --- CHANGELOG.md | 9 + doc/functions.inc.html | 8 + doc/mshell.md | 31 + mshell/BuiltInList.go | 7 + mshell/Evaluator.go | 219 +++++- mshell/Tar.go | 695 ++++++++++++++++++ mshell/TypeBuiltins.go | 16 + tests/success/tar_dir.msh | 24 + tests/success/tar_dir.msh.stdout | 14 + tests/success/tar_pack.msh | 29 + tests/success/tar_pack.msh.stdout | 13 + .../tarpack_entry_missing_path.msh | 3 + 12 files changed, 1066 insertions(+), 2 deletions(-) create mode 100644 mshell/Tar.go create mode 100644 tests/success/tar_dir.msh create mode 100644 tests/success/tar_dir.msh.stdout create mode 100644 tests/success/tar_pack.msh create mode 100644 tests/success/tar_pack.msh.stdout create mode 100644 tests/typecheck_fail/tarpack_entry_missing_path.msh diff --git a/CHANGELOG.md b/CHANGELOG.md index d984cf4..923ab74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 arbitrary base (2–36). `fromBase` returns `Maybe[int]`. - `toHex` / `toOctal` / `toBin` and `parseHex` / `parseOctal` / `parseBin`: convenience wrappers over `toBase` / `fromBase` for the common bases. + - `tarDirInc` / `tarDirExc` / `tarPack` / `tarList` / `tarExtract` / + `tarExtractEntry` / `tarRead`: create, list, extract, and read `.tar` + archives, mirroring the existing `zip*` functions (same argument order and + option dicts). Compression is chosen from the destination extension when + writing (`.tar.gz` / `.tgz` → gzip, `.tar` → uncompressed) and auto-detected + from the gzip magic bytes when reading, so `.tar.gz` is handled + transparently. Symlinks are preserved on pack and recreated on extract + (with a guard against targets escaping the destination); hard links and + device nodes are rejected. - Optional fields in dictionary shape types, written `name?: T` (and `"name"?: T` in `def` signatures). An optional field may be absent from a value; when present, its value is still type-checked. This lets option-style diff --git a/doc/functions.inc.html b/doc/functions.inc.html index 5a7edb3..2cd130d 100644 --- a/doc/functions.inc.html +++ b/doc/functions.inc.html @@ -126,6 +126,14 @@

File and Directory zipExtract Extract an entire archive. Options dict is required; defaults: overwrite=false, skipExisting=false (mutually exclusive), stripComponents=0, pattern="" (glob matched before stripping), preservePermissions=true. Destination is created if missing. (path:zipPath path:destDir dict:options -- ) zipExtractEntry Extract a single entry (file or directory subtree) to a destination path. Options dict is required; defaults: overwrite=false, skipExisting=false (mutually exclusive), preservePermissions=true, mkdirs=true (create parent directories when needed). (path:zipPath str:entry path:dest dict:options -- ) zipRead Read an entry’s bytes directly into the stack without writing to disk. Returns none when the entry does not exist. (path:zipPath str:entry -- Maybe[binary]) + Tar functions mirror the zip* surface (same argument order and option dicts). Compression is chosen from the destination extension when writing (.tar.gz / .tgz → gzip, .tar → uncompressed) and auto-detected from the gzip magic bytes when reading. Symlinks are preserved: packed as symlink entries and recreated on extraction (with an escape guard); hard links and device nodes are rejected. + tarDirInc Create/overwrite a .tar / .tar.gz from a directory; the archive root contains the directory’s contents (no parent folder). (path:sourceDir path:tarPath -- ) + tarDirExc Create/overwrite a .tar / .tar.gz that includes the source directory itself at the archive root (entries are prefixed with the directory name). (path:sourceDir path:tarPath -- ) + tarPack Create/overwrite a .tar / .tar.gz by packing a list of entries. Each entry is either a bare string/path (the file or directory to add, keeping its base name and mode) or a dictionary requiring path; in the dictionary form archivePath (override the in-archive name) and mode are optional. mode is a Go os.FileMode; write it with an octal literal, e.g. 0o644 (rw-r--r--), 0o755 (rwxr-xr-x), 0o600. If mode is omitted, the entry keeps the source file’s own mode. ([str|path|dict] path:tarPath -- ) + tarList List archive entries as dictionaries with keys: name (string, forward-slash paths, directories end with /), compressedSize and uncompressedSize (int bytes; equal, since tar has no per-entry compressed size), isDir (bool), perm (int POSIX permission bits), executable (bool), modified (datetime), type ("file"/"dir"/"symlink"), and linkTarget (symlink target, empty otherwise). (path -- [dict]) + tarExtract Extract an entire archive. Options dict is required; defaults: overwrite=false, skipExisting=false (mutually exclusive), stripComponents=0, pattern="" (glob matched before stripping), preservePermissions=true. Destination is created if missing. (path:tarPath path:destDir dict:options -- ) + tarExtractEntry Extract a single entry (file or directory subtree) to a destination path. Options dict is required; defaults: overwrite=false, skipExisting=false (mutually exclusive), preservePermissions=true, mkdirs=true (create parent directories when needed). (path:tarPath str:entry path:dest dict:options -- ) + tarRead Read an entry’s bytes directly into the stack without writing to disk. Returns none when the entry does not exist. (path:tarPath str:entry -- Maybe[binary]) readFile Read a file into a string. (str -- str) readFileBytes Read a file into binary data. (str -- binary) readTsvFile Read a TSV file into a list of rows. (str -- [[str]]) diff --git a/doc/mshell.md b/doc/mshell.md index 227d93e..492af17 100644 --- a/doc/mshell.md +++ b/doc/mshell.md @@ -1419,6 +1419,37 @@ See [Regexp.Expand](https://pkg.go.dev/regexp#Regexp.Expand) for replacement syn - `zipExtractEntry`: Extract a single entry (file or directory subtree) to a destination path. Options dict is required; defaults: `overwrite=false`, `skipExisting=false` (mutually exclusive), `preservePermissions=true`, `mkdirs=true`. `(path:zipPath str:entry path:dest dict:options -- )` - `zipRead`: Read an entry's bytes directly onto the stack without writing to disk. Returns `none` when the entry does not exist. `(path:zipPath str:entry -- Maybe[binary])` +## Archive (Tar) Functions + +The tar functions mirror the `zip*` surface exactly: same names with a `tar` +prefix, same argument order, and the same option dictionaries. +Two things differ, both driven by the tar format: + +- Compression is selected by the destination extension when writing: + `.tar.gz` or `.tgz` produce a gzip-compressed tarball, `.tar` is uncompressed. + When reading, the gzip magic bytes are auto-detected, so a gzipped tarball is + read transparently regardless of its filename. +- Symlinks are preserved: `tarPack`/`tarDir*` store symlinks as symlink entries, + and `tarExtract`/`tarExtractEntry` recreate them (rejecting any whose target + would escape the destination directory). Hard links and device/fifo nodes are + rejected with an error. + +- `tarDirInc`: Create/overwrite a `.tar`/`.tar.gz` from a directory; the archive root contains the directory's contents (no parent folder). `(path:sourceDir path:tarPath -- )` +- `tarDirExc`: Create/overwrite a `.tar`/`.tar.gz` that includes the source directory itself at the archive root (entries are prefixed with the directory name). `(path:sourceDir path:tarPath -- )` +- `tarPack`: Create/overwrite a `.tar`/`.tar.gz` by packing a list of entries. + Each entry is either a bare string/path (the file or directory to add, + keeping its base name and mode) or a dictionary. + Each dictionary entry requires `path` (the file or directory to add); + `archivePath` (override the in-archive name) and `mode` are optional. + `mode` is a Go `os.FileMode`; write it with an octal literal, + e.g. `0o644` (`rw-r--r--`), `0o755` (`rwxr-xr-x`), `0o600`. + If `mode` is omitted, the entry keeps the source file's own mode. + Type: `([str | path | {path: str | path, archivePath?: str | path, mode?: int}] str | path -- )` +- `tarList`: List archive entries as dictionaries with keys: `name` (string, forward-slash paths, directories end with `/`), `compressedSize` and `uncompressedSize` (int bytes; equal, since tar has no per-entry compressed size), `isDir` (bool), `perm` (int POSIX permission bits), `executable` (bool), `modified` (datetime from the archive entry), `type` (`"file"`/`"dir"`/`"symlink"`), and `linkTarget` (symlink target, empty otherwise). `(path -- [dict])` +- `tarExtract`: Extract an entire archive. Options dict is required; defaults: `overwrite=false`, `skipExisting=false` (mutually exclusive), `stripComponents=0`, `pattern=""` (glob matched before stripping), `preservePermissions=true`. Destination is created if missing. `(path:tarPath path:destDir dict:options -- )` +- `tarExtractEntry`: Extract a single entry (file or directory subtree) to a destination path. Options dict is required; defaults: `overwrite=false`, `skipExisting=false` (mutually exclusive), `preservePermissions=true`, `mkdirs=true`. `(path:tarPath str:entry path:dest dict:options -- )` +- `tarRead`: Read an entry's bytes directly onto the stack without writing to disk. Returns `none` when the entry does not exist. `(path:tarPath str:entry -- Maybe[binary])` + ## Variables You can store to several variables in one go by separating the store tokens with commas. diff --git a/mshell/BuiltInList.go b/mshell/BuiltInList.go index a73b0ff..ed666cb 100644 --- a/mshell/BuiltInList.go +++ b/mshell/BuiltInList.go @@ -221,6 +221,13 @@ var BuiltInList = map[string]struct{}{ "writeFile": {}, "wsplit": {}, "year": {}, + "tarDirExc": {}, + "tarDirInc": {}, + "tarExtract": {}, + "tarExtractEntry": {}, + "tarList": {}, + "tarPack": {}, + "tarRead": {}, "zipDirExc": {}, "zipDirInc": {}, "zipExtract": {}, diff --git a/mshell/Evaluator.go b/mshell/Evaluator.go index 2fd1412..a560e04 100644 --- a/mshell/Evaluator.go +++ b/mshell/Evaluator.go @@ -5083,7 +5083,7 @@ func parseZipExtractOptions(dict *MShellDict) (zipExtractOptions, error) { } if options.overwrite && options.skipExisting { - return options, fmt.Errorf("zipExtract options 'overwrite' and 'skipExisting' are mutually exclusive") + return options, fmt.Errorf("options 'overwrite' and 'skipExisting' are mutually exclusive") } if val, ok, err := boolOption(dict, "preservePermissions"); err != nil { @@ -5133,7 +5133,7 @@ func parseZipExtractEntryOptions(dict *MShellDict) (zipExtractEntryOptions, erro } if options.overwrite && options.skipExisting { - return options, fmt.Errorf("zipExtractEntry options 'overwrite' and 'skipExisting' are mutually exclusive") + return options, fmt.Errorf("options 'overwrite' and 'skipExisting' are mutually exclusive") } if val, ok, err := boolOption(dict, "preservePermissions"); err != nil { @@ -7715,6 +7715,221 @@ func (state *EvalState) evaluateToken(t Token, stack *MShellStack, context Execu } else { stack.Push(&Maybe{obj: MShellBinary(data)}) } + } else if t.Lexeme == "tarDirInc" || t.Lexeme == "tarDirExc" { + obj1, obj2, err := stack.Pop2(t) + if err != nil { + return state.FailWithMessage(err.Error()) + } + + tarPath, err := obj1.CastString() + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: Cannot tar into a %s.\n", t.Line, t.Column, obj1.TypeName())) + } + + sourceDir, err := obj2.CastString() + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: Cannot tar from a %s.\n", t.Line, t.Column, obj2.TypeName())) + } + + preserveRoot := t.Lexeme == "tarDirInc" + if err := tarDirectory(sourceDir, tarPath, preserveRoot); err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: %s\n", t.Line, t.Column, err.Error())) + } + } else if t.Lexeme == "tarPack" { + obj1, obj2, err := stack.Pop2(t) + if err != nil { + return state.FailWithMessage(err.Error()) + } + + tarPath, err := obj1.CastString() + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: Cannot tar into a %s.\n", t.Line, t.Column, obj1.TypeName())) + } + + list, ok := obj2.(*MShellList) + if !ok { + return state.FailWithMessage(fmt.Sprintf("%d:%d: tarPack expects a list of dictionaries describing the entries to add. Found %s.\n", t.Line, t.Column, obj2.TypeName())) + } + + entries := make([]zipPackItem, 0, len(list.Items)) + for idx, item := range list.Items { + entryDict, ok := item.(*MShellDict) + if !ok { + switch pathItem := item.(type) { + case MShellString: + entries = append(entries, zipPackItem{SourcePath: pathItem.Content, PreserveRoot: true}) + continue + case MShellPath: + entries = append(entries, zipPackItem{SourcePath: pathItem.Path, PreserveRoot: true}) + continue + } + return state.FailWithMessage(fmt.Sprintf("%d:%d: tarPack entry %d is not a string, path, or dictionary. Found %s.\n", t.Line, t.Column, idx, item.TypeName())) + } + + sourceObj, ok := entryDict.Items["path"] + if !ok { + return state.FailWithMessage(fmt.Sprintf("%d:%d: tarPack entry %d is missing required 'path'.\n", t.Line, t.Column, idx)) + } + + sourcePath, err := sourceObj.CastString() + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: tarPack entry %d had a non-string path (%s).\n", t.Line, t.Column, idx, sourceObj.TypeName())) + } + + packItem := zipPackItem{SourcePath: sourcePath, PreserveRoot: true} + if archiveObj, ok := entryDict.Items["archivePath"]; ok { + archivePath, err := archiveObj.CastString() + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: tarPack entry %d had an invalid archivePath (%s).\n", t.Line, t.Column, idx, archiveObj.TypeName())) + } + packItem.ArchivePath = archivePath + packItem.PreserveRoot = false + } + if modeObj, ok := entryDict.Items["mode"]; ok { + modeInt, ok := modeObj.(MShellInt) + if !ok { + return state.FailWithMessage(fmt.Sprintf("%d:%d: tarPack entry %d had a non-integer mode (%s).\n", t.Line, t.Column, idx, modeObj.TypeName())) + } + mode := os.FileMode(modeInt.Value) + packItem.ModeOverride = &mode + } + entries = append(entries, packItem) + } + + if err := buildTarFromEntries(entries, tarPath); err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: %s\n", t.Line, t.Column, err.Error())) + } + } else if t.Lexeme == "tarList" { + obj1, err := stack.Pop() + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: Cannot do 'tarList' operation on an empty stack.\n", t.Line, t.Column)) + } + + tarPath, err := obj1.CastString() + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: Cannot list entries in a %s.\n", t.Line, t.Column, obj1.TypeName())) + } + + entries, err := collectTarMetadata(tarPath) + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: %s\n", t.Line, t.Column, err.Error())) + } + + result := NewList(0) + for _, entry := range entries { + dict := NewDict() + dict.Items["name"] = MShellString{entry.Name} + dict.Items["compressedSize"] = MShellInt{entry.Size} + dict.Items["uncompressedSize"] = MShellInt{entry.Size} + dict.Items["isDir"] = MShellBool{entry.IsDir} + dict.Items["perm"] = MShellInt{int(entry.Mode.Perm())} + dict.Items["executable"] = MShellBool{entry.Type == "file" && entry.Mode.Perm()&0o111 != 0} + dict.Items["modified"] = &MShellDateTime{Time: entry.Modified, OriginalString: entry.Modified.Format("2006-01-02T15:04:05")} + dict.Items["type"] = MShellString{entry.Type} + dict.Items["linkTarget"] = MShellString{entry.LinkTarget} + result.Items = append(result.Items, dict) + } + stack.Push(result) + } else if t.Lexeme == "tarExtract" { + obj1, obj2, obj3, err := stack.Pop3(t) + if err != nil { + return state.FailWithMessage(err.Error()) + } + + optionsDict, ok := obj1.(*MShellDict) + if !ok { + return state.FailWithMessage(fmt.Sprintf("%d:%d: tarExtract expects an options dictionary. Found %s.\n", t.Line, t.Column, obj1.TypeName())) + } + + destDir, err := obj2.CastString() + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: tarExtract destination must be a string/path. Found %s.\n", t.Line, t.Column, obj2.TypeName())) + } + + tarPath, err := obj3.CastString() + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: tarExtract source must be a string/path. Found %s.\n", t.Line, t.Column, obj3.TypeName())) + } + + options, err := parseZipExtractOptions(optionsDict) + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: %s\n", t.Line, t.Column, err.Error())) + } + + if err := extractTarArchive(tarPath, destDir, options); err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: %s\n", t.Line, t.Column, err.Error())) + } + } else if t.Lexeme == "tarExtractEntry" { + optionsObj, err := stack.Pop() + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: Cannot do 'tarExtractEntry' operation on an empty stack.\n", t.Line, t.Column)) + } + destObj, err := stack.Pop() + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: tarExtractEntry requires four arguments.\n", t.Line, t.Column)) + } + entryObj, err := stack.Pop() + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: tarExtractEntry requires four arguments.\n", t.Line, t.Column)) + } + tarObj, err := stack.Pop() + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: tarExtractEntry requires four arguments.\n", t.Line, t.Column)) + } + + optionsDict, ok := optionsObj.(*MShellDict) + if !ok { + return state.FailWithMessage(fmt.Sprintf("%d:%d: tarExtractEntry expects an options dictionary. Found %s.\n", t.Line, t.Column, optionsObj.TypeName())) + } + + destPath, err := destObj.CastString() + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: tarExtractEntry destination must be a string/path. Found %s.\n", t.Line, t.Column, destObj.TypeName())) + } + + entryPath, err := entryObj.CastString() + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: tarExtractEntry entry name must be a string/path. Found %s.\n", t.Line, t.Column, entryObj.TypeName())) + } + + tarPath, err := tarObj.CastString() + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: tarExtractEntry source must be a string/path. Found %s.\n", t.Line, t.Column, tarObj.TypeName())) + } + + options, err := parseZipExtractEntryOptions(optionsDict) + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: %s\n", t.Line, t.Column, err.Error())) + } + + if err := extractTarEntry(tarPath, entryPath, destPath, options); err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: %s\n", t.Line, t.Column, err.Error())) + } + } else if t.Lexeme == "tarRead" { + obj1, obj2, err := stack.Pop2(t) + if err != nil { + return state.FailWithMessage(err.Error()) + } + + entryPath, err := obj1.CastString() + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: tarRead entry name must be a string/path. Found %s.\n", t.Line, t.Column, obj1.TypeName())) + } + + tarPath, err := obj2.CastString() + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: tarRead archive path must be a string/path. Found %s.\n", t.Line, t.Column, obj2.TypeName())) + } + + data, found, err := readTarEntry(tarPath, entryPath) + if err != nil { + return state.FailWithMessage(fmt.Sprintf("%d:%d: %s\n", t.Line, t.Column, err.Error())) + } + if !found { + stack.Push(&Maybe{obj: nil}) + } else { + stack.Push(&Maybe{obj: MShellBinary(data)}) + } } else if t.Lexeme == "e" || t.Lexeme == "ec" || t.Lexeme == "es" { // Token Type obj, err := stack.Pop() diff --git a/mshell/Tar.go b/mshell/Tar.go new file mode 100644 index 0000000..0fc254b --- /dev/null +++ b/mshell/Tar.go @@ -0,0 +1,695 @@ +package main + +import ( + "archive/tar" + "bufio" + "compress/gzip" + "errors" + "fmt" + "io" + "io/fs" + "os" + "path" + "path/filepath" + "strings" + "time" +) + +// tarEntryMetadata mirrors zipEntryMetadata but carries the tar-specific +// entry type and symlink target. tar has no per-entry compressed size, so +// tarList reports compressedSize == uncompressedSize (documented). +type tarEntryMetadata struct { + Name string + Size int + Modified time.Time + IsDir bool + Mode os.FileMode + Type string // "file", "dir", or "symlink" + LinkTarget string +} + +// funcCloser adapts a plain function to io.Closer so the gzip + file layers +// can be torn down together. +type funcCloser func() error + +func (f funcCloser) Close() error { return f() } + +// isGzipTarget reports whether a destination path should be gzip-compressed +// based on its extension (.gz or .tgz). Matches `tar -a` auto-compression. +func isGzipTarget(tarPath string) bool { + lower := strings.ToLower(tarPath) + return strings.HasSuffix(lower, ".gz") || strings.HasSuffix(lower, ".tgz") +} + +// openTarReader opens a tar archive for reading, transparently decompressing +// gzip streams by sniffing the magic bytes (0x1f 0x8b) regardless of the file +// extension. The returned Closer tears down every layer that was opened. +func openTarReader(tarPath string) (*tar.Reader, io.Closer, error) { + file, err := os.Open(tarPath) + if err != nil { + return nil, nil, fmt.Errorf("Error opening %s: %w", tarPath, err) + } + + br := bufio.NewReader(file) + magic, err := br.Peek(2) + if err != nil && err != io.EOF { + file.Close() + return nil, nil, fmt.Errorf("Error reading %s: %w", tarPath, err) + } + + if len(magic) == 2 && magic[0] == 0x1f && magic[1] == 0x8b { + gz, err := gzip.NewReader(br) + if err != nil { + file.Close() + return nil, nil, fmt.Errorf("Error reading gzip stream %s: %w", tarPath, err) + } + closer := funcCloser(func() error { + gz.Close() + return file.Close() + }) + return tar.NewReader(gz), closer, nil + } + + return tar.NewReader(br), file, nil +} + +// createTarWriter creates a tar archive for writing, gzip-compressing when the +// destination extension calls for it. The returned finish function must be +// called (not deferred with a discarded error) to flush and close every layer. +func createTarWriter(tarPath string) (*tar.Writer, func() error, error) { + if err := os.MkdirAll(filepath.Dir(tarPath), 0755); err != nil { + return nil, nil, fmt.Errorf("Error creating parent directory for %s: %w", tarPath, err) + } + + output, err := os.Create(tarPath) + if err != nil { + return nil, nil, fmt.Errorf("Error creating %s: %w", tarPath, err) + } + + if isGzipTarget(tarPath) { + gz := gzip.NewWriter(output) + tw := tar.NewWriter(gz) + finish := func() error { + if err := tw.Close(); err != nil { + output.Close() + return fmt.Errorf("Error finalizing tar %s: %w", tarPath, err) + } + if err := gz.Close(); err != nil { + output.Close() + return fmt.Errorf("Error finalizing gzip %s: %w", tarPath, err) + } + return output.Close() + } + return tw, finish, nil + } + + tw := tar.NewWriter(output) + finish := func() error { + if err := tw.Close(); err != nil { + output.Close() + return fmt.Errorf("Error finalizing tar %s: %w", tarPath, err) + } + return output.Close() + } + return tw, finish, nil +} + +// tarDirectory packs a single directory into a tarball, mirroring zipDirectory. +// preserveRoot controls whether the directory itself appears at the archive +// root (tarDirExc) or only its contents (tarDirInc). +func tarDirectory(sourceDir, tarPath string, preserveRoot bool) error { + info, err := os.Stat(sourceDir) + if err != nil { + return fmt.Errorf("Error stating %s: %w", sourceDir, err) + } + if !info.IsDir() { + return fmt.Errorf("tarDir expects a directory. %s is not a directory", sourceDir) + } + + srcAbs, err := filepath.Abs(sourceDir) + if err != nil { + return fmt.Errorf("Error resolving %s: %w", sourceDir, err) + } + if err := ensureTarTargetNotInsideSource(srcAbs, tarPath); err != nil { + return err + } + + packItem := zipPackItem{ + SourcePath: sourceDir, + PreserveRoot: preserveRoot, + } + return buildTarFromEntries([]zipPackItem{packItem}, tarPath) +} + +func ensureTarTargetNotInsideSource(sourceAbs string, tarPath string) error { + tarAbs, err := filepath.Abs(tarPath) + if err != nil { + return fmt.Errorf("Error resolving destination %s: %w", tarPath, err) + } + sourceWithSep := ensureTrailingSeparator(sourceAbs) + if tarAbs == sourceAbs || strings.HasPrefix(tarAbs, sourceWithSep) { + return fmt.Errorf("Tar destination %s cannot be inside the source directory %s", tarPath, sourceAbs) + } + return nil +} + +// buildTarFromEntries mirrors buildZipFromEntries, reusing the zipPackItem +// model so the tarPack dispatch parsing is identical to zipPack. +func buildTarFromEntries(items []zipPackItem, tarPath string) error { + if len(items) == 0 { + return fmt.Errorf("tarPack requires at least one entry") + } + + tarAbs, err := filepath.Abs(tarPath) + if err != nil { + return fmt.Errorf("Error resolving %s: %w", tarPath, err) + } + + tw, finish, err := createTarWriter(tarPath) + if err != nil { + return err + } + + for _, item := range items { + info, err := os.Lstat(item.SourcePath) + if err != nil { + finish() + return fmt.Errorf("Error stating %s: %w", item.SourcePath, err) + } + + sourceAbs, err := filepath.Abs(item.SourcePath) + if err != nil { + finish() + return fmt.Errorf("Error resolving %s: %w", item.SourcePath, err) + } + sourceAbsWithSep := ensureTrailingSeparator(sourceAbs) + if tarAbs == sourceAbs || strings.HasPrefix(tarAbs, sourceAbsWithSep) { + finish() + return fmt.Errorf("Tar destination %s cannot be inside the source path %s", tarPath, sourceAbs) + } + + if info.IsDir() { + prefix := strings.Trim(item.ArchivePath, "/") + if prefix == "" && item.PreserveRoot { + prefix = filepath.Base(sourceAbs) + } + if err := addDirectoryToTar(tw, item.SourcePath, prefix, item.ModeOverride); err != nil { + finish() + return err + } + continue + } + + name := item.ArchivePath + if name == "" { + name = filepath.Base(item.SourcePath) + } + name = strings.Trim(name, "/") + if name == "" { + finish() + return fmt.Errorf("tarPack entry for %s produced an empty archive path", item.SourcePath) + } + + if err := addFileToTar(tw, item.SourcePath, name, info, item.ModeOverride); err != nil { + finish() + return err + } + } + + if err := finish(); err != nil { + return err + } + return nil +} + +func addDirectoryToTar(tw *tar.Writer, sourcePath, archivePrefix string, modeOverride *os.FileMode) error { + cleanPrefix := strings.Trim(archivePrefix, "/") + return filepath.WalkDir(sourcePath, func(pathStr string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + + info, err := d.Info() + if err != nil { + return err + } + + relPath, err := filepath.Rel(sourcePath, pathStr) + if err != nil { + return err + } + relPath = filepath.ToSlash(relPath) + + var entryName string + if relPath == "." { + entryName = cleanPrefix + } else if cleanPrefix == "" { + entryName = relPath + } else { + entryName = path.Join(cleanPrefix, relPath) + } + + entryName = strings.Trim(entryName, "/") + if entryName == "" { + // Skip the implicit root when no prefix is requested. + return nil + } + if info.IsDir() { + entryName += "/" + } + + return addFileToTar(tw, pathStr, entryName, info, modeOverride) + }) +} + +// addFileToTar writes a single filesystem entry into the tar stream. +// Regular files, directories, and symlinks are supported; symlinks are stored +// as symlinks (preserving the target) rather than being dereferenced. +func addFileToTar(tw *tar.Writer, sourcePath, entryName string, info os.FileInfo, modeOverride *os.FileMode) error { + linkTarget := "" + if info.Mode()&os.ModeSymlink != 0 { + target, err := os.Readlink(sourcePath) + if err != nil { + return fmt.Errorf("Error reading symlink %s: %w", sourcePath, err) + } + linkTarget = target + } + + header, err := tar.FileInfoHeader(info, linkTarget) + if err != nil { + return err + } + header.Name = path.Clean(strings.ReplaceAll(entryName, "\\", "/")) + if info.IsDir() && !strings.HasSuffix(header.Name, "/") { + header.Name += "/" + } + if modeOverride != nil { + header.Mode = int64(modeOverride.Perm()) + } + + if err := tw.WriteHeader(header); err != nil { + return err + } + + if info.Mode()&os.ModeSymlink != 0 || info.IsDir() { + return nil + } + if !info.Mode().IsRegular() { + return fmt.Errorf("tarPack cannot archive %s: unsupported file type", sourcePath) + } + + file, err := os.Open(sourcePath) + if err != nil { + return err + } + defer file.Close() + + if _, err := io.Copy(tw, file); err != nil { + return err + } + return nil +} + +func tarEntryType(header *tar.Header) string { + switch header.Typeflag { + case tar.TypeDir: + return "dir" + case tar.TypeSymlink: + return "symlink" + default: + return "file" + } +} + +func collectTarMetadata(tarPath string) ([]tarEntryMetadata, error) { + reader, closer, err := openTarReader(tarPath) + if err != nil { + return nil, err + } + defer closer.Close() + + entries := make([]tarEntryMetadata, 0) + for { + header, err := reader.Next() + if err == io.EOF { + break + } + if err != nil { + return nil, fmt.Errorf("Error reading %s: %w", tarPath, err) + } + + size, err := safeSizeToInt(uint64(header.Size)) + if err != nil { + return nil, err + } + + info := header.FileInfo() + entries = append(entries, tarEntryMetadata{ + Name: header.Name, + Size: size, + Modified: header.ModTime, + IsDir: info.IsDir(), + Mode: info.Mode(), + Type: tarEntryType(header), + LinkTarget: header.Linkname, + }) + } + + return entries, nil +} + +// extractTarArchive extracts an entire tarball, honoring the same option set as +// extractZipArchive (overwrite/skipExisting/stripComponents/pattern/ +// preservePermissions). Symlinks are recreated with an escape guard. +func extractTarArchive(tarPath, destDir string, options zipExtractOptions) error { + reader, closer, err := openTarReader(tarPath) + if err != nil { + return err + } + defer closer.Close() + + absDest, err := filepath.Abs(destDir) + if err != nil { + return fmt.Errorf("Error resolving destination %s: %w", destDir, err) + } + if err := os.MkdirAll(absDest, 0755); err != nil { + return fmt.Errorf("Error creating destination %s: %w", absDest, err) + } + baseWithSep := ensureTrailingSeparator(absDest) + + for { + header, err := reader.Next() + if err == io.EOF { + break + } + if err != nil { + return fmt.Errorf("Error reading %s: %w", tarPath, err) + } + + entryName := normalizeZipEntryName(header.Name) + if entryName == "" { + continue + } + + if options.pattern != "" { + match, err := path.Match(options.pattern, entryName) + if err != nil { + return fmt.Errorf("Invalid tarExtract pattern '%s': %w", options.pattern, err) + } + if !match { + continue + } + } + + stripped, err := stripZipComponents(entryName, options.stripComponents, header.FileInfo().IsDir()) + if err != nil { + return err + } + if stripped == "" { + continue + } + + target := filepath.Join(absDest, filepath.FromSlash(stripped)) + target = filepath.Clean(target) + if err := ensureWithinBase(target, absDest, baseWithSep); err != nil { + return err + } + + if err := writeTarEntryToDisk(reader, header, target, options.zipWriteOptions, true, absDest, baseWithSep); err != nil { + return err + } + } + + return nil +} + +// extractTarEntry extracts a single named entry (a file or a directory subtree) +// mirroring extractZipEntry. Because tar is a stream format it makes a single +// pass, collecting the file entry or the subtree as it goes. +func extractTarEntry(tarPath, entryPath, destPath string, options zipExtractEntryOptions) error { + targetName := normalizeZipEntryName(entryPath) + if targetName == "" { + return fmt.Errorf("Entry path %s resolves to an empty name", entryPath) + } + + reader, closer, err := openTarReader(tarPath) + if err != nil { + return err + } + defer closer.Close() + + prefix := targetName + "/" + + absDest, err := filepath.Abs(destPath) + if err != nil { + return fmt.Errorf("Error resolving destination %s: %w", destPath, err) + } + + fileFound := false + dirCreated := false + baseWithSep := "" + + for { + header, err := reader.Next() + if err == io.EOF { + break + } + if err != nil { + return fmt.Errorf("Error reading %s: %w", tarPath, err) + } + + name := normalizeZipEntryName(header.Name) + + if name == targetName && !header.FileInfo().IsDir() { + // Single file entry: dest is the file path. + if err := extractTarSingleFile(reader, header, absDest, options); err != nil { + return err + } + fileFound = true + break + } + + isSelfDir := name == targetName && header.FileInfo().IsDir() + isChild := strings.HasPrefix(name, prefix) + if !isSelfDir && !isChild { + continue + } + + // Directory subtree: dest is a directory that receives the subtree. + if !dirCreated { + if options.mkdirs { + if err := os.MkdirAll(absDest, 0755); err != nil { + return fmt.Errorf("Error creating destination %s: %w", absDest, err) + } + } else { + info, err := os.Stat(absDest) + if err != nil { + return fmt.Errorf("Destination %s does not exist", absDest) + } + if !info.IsDir() { + return fmt.Errorf("Destination %s is not a directory", absDest) + } + } + baseWithSep = ensureTrailingSeparator(absDest) + dirCreated = true + } + + if isSelfDir { + continue + } + + relative := strings.TrimPrefix(name, prefix) + if relative == "" { + continue + } + + target := filepath.Join(absDest, filepath.FromSlash(relative)) + target = filepath.Clean(target) + if err := ensureWithinBase(target, absDest, baseWithSep); err != nil { + return err + } + + if err := writeTarEntryToDisk(reader, header, target, options.zipWriteOptions, true, absDest, baseWithSep); err != nil { + return err + } + } + + if !fileFound && !dirCreated { + return fmt.Errorf("Entry '%s' not found in %s", entryPath, tarPath) + } + return nil +} + +func extractTarSingleFile(reader *tar.Reader, header *tar.Header, absDest string, options zipExtractEntryOptions) error { + parent := filepath.Dir(absDest) + if options.mkdirs { + if err := os.MkdirAll(parent, 0755); err != nil { + return fmt.Errorf("Error creating parent directory %s: %w", parent, err) + } + } else { + if _, err := os.Stat(parent); err != nil { + return fmt.Errorf("Parent directory %s does not exist", parent) + } + } + + baseWithSep := ensureTrailingSeparator(parent) + return writeTarEntryToDisk(reader, header, absDest, options.zipWriteOptions, false, parent, baseWithSep) +} + +// writeTarEntryToDisk writes one tar entry (directory, file, or symlink) to +// destPath. For symlinks it validates that the link target cannot escape the +// destination root before creating it. +func writeTarEntryToDisk(reader *tar.Reader, header *tar.Header, destPath string, options zipWriteOptions, ensureParents bool, base, baseWithSep string) error { + switch header.Typeflag { + case tar.TypeDir: + if ensureParents { + if err := os.MkdirAll(destPath, 0755); err != nil { + return fmt.Errorf("Error creating directory %s: %w", destPath, err) + } + } else if err := os.Mkdir(destPath, 0755); err != nil && !errors.Is(err, os.ErrExist) { + return fmt.Errorf("Error creating directory %s: %w", destPath, err) + } + if options.preservePermissions { + if err := os.Chmod(destPath, header.FileInfo().Mode().Perm()); err != nil && !errors.Is(err, os.ErrPermission) { + return fmt.Errorf("Error setting permissions on %s: %w", destPath, err) + } + } + return nil + case tar.TypeSymlink: + return writeTarSymlink(header, destPath, options, ensureParents, base, baseWithSep) + case tar.TypeReg, '\x00': // '\x00' is the legacy TypeRegA (deprecated) regular-file flag + return writeTarRegularFile(reader, header, destPath, options, ensureParents) + default: + return fmt.Errorf("tarExtract cannot handle entry %s (unsupported type %q)", header.Name, string(header.Typeflag)) + } +} + +func writeTarSymlink(header *tar.Header, destPath string, options zipWriteOptions, ensureParents bool, base, baseWithSep string) error { + // Resolve the link target relative to the symlink's own directory and + // ensure it stays within the destination root. + linkDir := filepath.Dir(destPath) + var resolved string + if filepath.IsAbs(header.Linkname) { + resolved = filepath.Clean(header.Linkname) + } else { + resolved = filepath.Clean(filepath.Join(linkDir, filepath.FromSlash(header.Linkname))) + } + if err := ensureWithinBase(resolved, base, baseWithSep); err != nil { + return fmt.Errorf("Refusing to extract symlink %s pointing outside destination (%s)", header.Name, header.Linkname) + } + + parentDir := filepath.Dir(destPath) + if ensureParents { + if err := os.MkdirAll(parentDir, 0755); err != nil { + return fmt.Errorf("Error creating parent directory %s: %w", parentDir, err) + } + } else if _, err := os.Stat(parentDir); err != nil { + return fmt.Errorf("Parent directory %s does not exist", parentDir) + } + + if _, err := os.Lstat(destPath); err == nil { + if options.skipExisting { + return nil + } + if !options.overwrite { + return fmt.Errorf("Destination %s already exists", destPath) + } + if err := os.Remove(destPath); err != nil { + return fmt.Errorf("Error replacing %s: %w", destPath, err) + } + } else if !errors.Is(err, os.ErrNotExist) { + return err + } + + if err := os.Symlink(header.Linkname, destPath); err != nil { + return fmt.Errorf("Error creating symlink %s: %w", destPath, err) + } + return nil +} + +func writeTarRegularFile(reader *tar.Reader, header *tar.Header, destPath string, options zipWriteOptions, ensureParents bool) error { + parentDir := filepath.Dir(destPath) + if ensureParents { + if err := os.MkdirAll(parentDir, 0755); err != nil { + return fmt.Errorf("Error creating parent directory %s: %w", parentDir, err) + } + } else { + if _, err := os.Stat(parentDir); err != nil { + return fmt.Errorf("Parent directory %s does not exist", parentDir) + } + } + + if options.skipExisting { + if _, err := os.Stat(destPath); err == nil { + return nil + } else if !errors.Is(err, os.ErrNotExist) { + return err + } + } else { + if _, err := os.Stat(destPath); err == nil && !options.overwrite { + return fmt.Errorf("Destination %s already exists", destPath) + } else if err != nil && !errors.Is(err, os.ErrNotExist) { + return err + } + } + + mode := header.FileInfo().Mode().Perm() + outFile, err := os.OpenFile(destPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, mode) + if err != nil { + return err + } + defer outFile.Close() + + if _, err := io.Copy(outFile, reader); err != nil { + return err + } + + if options.preservePermissions { + if err := os.Chmod(destPath, mode); err != nil && !errors.Is(err, os.ErrPermission) { + return fmt.Errorf("Error setting permissions on %s: %w", destPath, err) + } + } + return nil +} + +// readTarEntry reads a single entry's bytes without writing to disk, mirroring +// readZipEntry. Returns found=false when the entry is absent. +func readTarEntry(tarPath, entryPath string) ([]byte, bool, error) { + target := normalizeZipEntryName(entryPath) + if target == "" { + return nil, false, fmt.Errorf("Entry path %s resolves to an empty name", entryPath) + } + + reader, closer, err := openTarReader(tarPath) + if err != nil { + return nil, false, err + } + defer closer.Close() + + for { + header, err := reader.Next() + if err == io.EOF { + break + } + if err != nil { + return nil, false, fmt.Errorf("Error reading %s: %w", tarPath, err) + } + + name := normalizeZipEntryName(header.Name) + if name != target { + continue + } + + if header.FileInfo().IsDir() { + return nil, false, fmt.Errorf("tarRead cannot read directory entries (%s)", entryPath) + } + + data, err := io.ReadAll(reader) + if err != nil { + return nil, false, err + } + return data, true, nil + } + + return nil, false, nil +} diff --git a/mshell/TypeBuiltins.go b/mshell/TypeBuiltins.go index d67521e..6ab1b41 100644 --- a/mshell/TypeBuiltins.go +++ b/mshell/TypeBuiltins.go @@ -608,6 +608,22 @@ func builtinSigsByName(arena *TypeArena, names *NameTable) map[NameId][]QuoteSig // type-check without forcing every call site to add `str`. r.reg("zipList", "(str | path -- [{str: str}])") + // Tar ops mirror the zip surface exactly (same argument order and option + // dicts). Compression is chosen from the destination extension on write + // (.tar.gz / .tgz -> gzip) and sniffed from the gzip magic bytes on read. + r.reg("tarRead", "(str | path str | path -- Maybe[bytes])") + for _, name := range []string{"tarDirInc", "tarDirExc"} { + r.reg(name, "(str | path str | path -- )") + } + r.reg("tarPack", "([str | path | {path: str | path, archivePath?: str | path, mode?: int}] str | path -- )") + r.reg("tarExtract", "(str | path str | path "+zipExtractOpts+" -- )") + r.reg("tarExtractEntry", + "(str str str "+zipEntryOpts+" -- )", + "(path str path "+zipEntryOpts+" -- )", + ) + // tarList: same widened string-valued metadata modeling as zipList. + r.reg("tarList", "(str | path -- [{str: str}])") + // groupBy list form: bucket by a str key. r.reg("groupBy", "([t] (t -- str) -- {[t]})") // groupBy grid form: diff --git a/tests/success/tar_dir.msh b/tests/success/tar_dir.msh new file mode 100644 index 0000000..4521cc0 --- /dev/null +++ b/tests/success/tar_dir.msh @@ -0,0 +1,24 @@ +# tarDir basic coverage plus listing, tarRead, and gzip round-trip. +tempFile newTarName! +`zip/base` @newTarName tarDirExc + +"tarDir entries" wl +@newTarName tarList tarEntries! +@tarEntries :0: keys ("modified" !=) filter keys! # Filter out modified because that breaks unit tests +@keys tjoin wl +@tarEntries ( + entry! @keys (@entry swap get? str) map +) map tuw + +"tarDir alpha" wl +@newTarName "alpha.txt" tarRead ? utf8Str wl + +"tarDir missing isNone" wl +@newTarName "missing.txt" tarRead isNone str wl + +# Gzip round-trip: pack to a .tar.gz, then read back through magic-byte sniffing. +".tar.gz" tempFileExt gzName! +`zip/base` @gzName tarDirExc + +"tarGz alpha" wl +@gzName "alpha.txt" tarRead ? utf8Str wl diff --git a/tests/success/tar_dir.msh.stdout b/tests/success/tar_dir.msh.stdout new file mode 100644 index 0000000..6021497 --- /dev/null +++ b/tests/success/tar_dir.msh.stdout @@ -0,0 +1,14 @@ +tarDir entries +compressedSize executable isDir linkTarget name perm type uncompressedSize +11 false false alpha.txt 420 file 11 +0 false true nested/ 493 dir 0 +11 false false nested/bravo.txt 420 file 11 +60 false false repeat.txt 420 file 60 +tarDir alpha +ALPHA-DATA + +tarDir missing isNone +true +tarGz alpha +ALPHA-DATA + diff --git a/tests/success/tar_pack.msh b/tests/success/tar_pack.msh new file mode 100644 index 0000000..9efdbc8 --- /dev/null +++ b/tests/success/tar_pack.msh @@ -0,0 +1,29 @@ +# tarPack custom entry coverage (mirrors zip_pack). +tempFile pack-tar! + +[ + { + path: `zip/base/alpha.txt`, + archivePath: "packed/alpha.txt", + mode: 420 # '0b110100100' + } + { + path: `zip/extra/extra.txt`, + archivePath: "extras/extra.txt", + mode: 493 + } + `zip/base/repeat.txt` + "zip/base/nested" +] pack-items! + +@pack-items @pack-tar tarPack + +"tarPack entries" wl +@pack-tar tarList (i! [@i "name" get? @i "perm" get? str @i "type" get? @i "executable" get? str] tjoin) map sortV uw +'' wl + +"tarPack alpha" wl +@pack-tar "packed/alpha.txt" tarRead ? utf8Str wl + +"tarPack extra" wl +@pack-tar "extras/extra.txt" tarRead ? utf8Str wl diff --git a/tests/success/tar_pack.msh.stdout b/tests/success/tar_pack.msh.stdout new file mode 100644 index 0000000..21d9049 --- /dev/null +++ b/tests/success/tar_pack.msh.stdout @@ -0,0 +1,13 @@ +tarPack entries +extras/extra.txt 493 file true +nested/ 493 dir false +nested/bravo.txt 420 file false +packed/alpha.txt 420 file false +repeat.txt 420 file false + +tarPack alpha +ALPHA-DATA + +tarPack extra +EXTRA-DATA + diff --git a/tests/typecheck_fail/tarpack_entry_missing_path.msh b/tests/typecheck_fail/tarpack_entry_missing_path.msh new file mode 100644 index 0000000..9909e5b --- /dev/null +++ b/tests/typecheck_fail/tarpack_entry_missing_path.msh @@ -0,0 +1,3 @@ +# Re-typed builtin: tarPack entries require `path`; an entry with only the +# optional `archivePath` must be rejected. +[ { 'archivePath': "packed.txt" } ] `out.tar` tarPack From 488026aff9349bb68f7cdd5ca8248196ddfff360 Mon Sep 17 00:00:00 2001 From: Mitchell Paulus Date: Tue, 7 Jul 2026 10:24:12 -0500 Subject: [PATCH 2/5] Harden tar/zip archive extraction against adversarial inputs Follow-up to the tar builtins: an adversarial sweep of the archive code surfaced hang and path-escape vectors. Fix them, and mirror the applicable fixes to the existing zip extractor so the two stay in lockstep. Each case is covered by a test (Tar_test.go, Tar_unix_test.go, ZipHardening_test.go). - Never hang: tarPack rejects fifo/device/socket entries before os.Open, and packing never follows source symlinks, so a symlink loop or a link to /dev/zero cannot block or inflate the archive. - Write-through a pre-existing symlink in the destination is now refused: ensureRealParentWithinBase resolves the deepest existing ancestor via EvalSymlinks and rejects when it lands outside the destination root, while still allowing symlinks that stay inside it. This closes a path-traversal vector present in both the tar and zip extractors. - Decompression-bomb cap: new optional maxBytes key (int, default 0 = unlimited) on the zip/tar extract option dicts bounds the total uncompressed bytes written per extraction via a shared streaming byteBudget. Verified against symlink loops, fifo packing, path traversal, escaping and pre-existing symlinks, hardlinks, truncated/garbage/empty archives, PAX/GNU long names, and a 200MB gzip bomb. Docs and CHANGELOG updated. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 8 ++ doc/functions.inc.html | 8 +- doc/mshell.md | 18 ++- mshell/Evaluator.go | 100 +++++++++++++- mshell/Tar.go | 43 +++--- mshell/Tar_test.go | 262 ++++++++++++++++++++++++++++++++++++ mshell/Tar_unix_test.go | 43 ++++++ mshell/TypeBuiltins.go | 4 +- mshell/ZipHardening_test.go | 79 +++++++++++ 9 files changed, 533 insertions(+), 32 deletions(-) create mode 100644 mshell/Tar_test.go create mode 100644 mshell/Tar_unix_test.go create mode 100644 mshell/ZipHardening_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 923ab74..6bc8650 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 transparently. Symlinks are preserved on pack and recreated on extract (with a guard against targets escaping the destination); hard links and device nodes are rejected. +- Optional `maxBytes` key (int, default `0` = unlimited) on the `zipExtract` / + `zipExtractEntry` / `tarExtract` / `tarExtractEntry` options dict: caps the + total uncompressed bytes written during an extraction to guard against + decompression bombs. +- Archive extraction (both `zip*` and `tar*`) now refuses to write through a + symlink that already exists in the destination directory and points outside + it, closing a path-traversal vector when extracting into a directory that + contains symlinks. - Optional fields in dictionary shape types, written `name?: T` (and `"name"?: T` in `def` signatures). An optional field may be absent from a value; when present, its value is still type-checked. This lets option-style diff --git a/doc/functions.inc.html b/doc/functions.inc.html index 2cd130d..6655dc0 100644 --- a/doc/functions.inc.html +++ b/doc/functions.inc.html @@ -123,16 +123,16 @@

File and Directory zipDirExc Create/overwrite a .zip that includes the source directory itself at the archive root (entries are prefixed with the directory name). (path:sourceDir path:zipPath -- ) zipPack Create/overwrite a .zip by packing a list of entries. Each entry is either a bare string/path (the file or directory to add, keeping its base name and mode) or a dictionary requiring path; in the dictionary form archivePath (override the in-archive name) and mode are optional. mode is a Go os.FileMode; write it with an octal literal, e.g. 0o644 (rw-r--r--), 0o755 (rwxr-xr-x), 0o600. On Linux/macOS these are the POSIX permission bits restored on extraction; on Windows file permissions are synthesized by Go and largely ignored (the executable bit is still preserved for Unix consumers). If mode is omitted, the entry keeps the source file’s own mode. ([str|path|dict] path:zipPath -- ) zipList List archive entries as dictionaries with columns: name (string, forward-slash paths, directories end with /), compressedSize (int bytes), uncompressedSize (int bytes), isDir (bool), perm (int POSIX permission bits), executable (bool), and modified (datetime from the archive entry). (path -- [dict]) - zipExtract Extract an entire archive. Options dict is required; defaults: overwrite=false, skipExisting=false (mutually exclusive), stripComponents=0, pattern="" (glob matched before stripping), preservePermissions=true. Destination is created if missing. (path:zipPath path:destDir dict:options -- ) - zipExtractEntry Extract a single entry (file or directory subtree) to a destination path. Options dict is required; defaults: overwrite=false, skipExisting=false (mutually exclusive), preservePermissions=true, mkdirs=true (create parent directories when needed). (path:zipPath str:entry path:dest dict:options -- ) + zipExtract Extract an entire archive. Options dict is required; defaults: overwrite=false, skipExisting=false (mutually exclusive), stripComponents=0, pattern="" (glob matched before stripping), preservePermissions=true, maxBytes=0 (0 = unlimited; a cap on the total uncompressed bytes written, guarding against decompression bombs). Destination is created if missing. (path:zipPath path:destDir dict:options -- ) + zipExtractEntry Extract a single entry (file or directory subtree) to a destination path. Options dict is required; defaults: overwrite=false, skipExisting=false (mutually exclusive), preservePermissions=true, mkdirs=true (create parent directories when needed), maxBytes=0 (0 = unlimited uncompressed-byte cap). (path:zipPath str:entry path:dest dict:options -- ) zipRead Read an entry’s bytes directly into the stack without writing to disk. Returns none when the entry does not exist. (path:zipPath str:entry -- Maybe[binary]) Tar functions mirror the zip* surface (same argument order and option dicts). Compression is chosen from the destination extension when writing (.tar.gz / .tgz → gzip, .tar → uncompressed) and auto-detected from the gzip magic bytes when reading. Symlinks are preserved: packed as symlink entries and recreated on extraction (with an escape guard); hard links and device nodes are rejected. tarDirInc Create/overwrite a .tar / .tar.gz from a directory; the archive root contains the directory’s contents (no parent folder). (path:sourceDir path:tarPath -- ) tarDirExc Create/overwrite a .tar / .tar.gz that includes the source directory itself at the archive root (entries are prefixed with the directory name). (path:sourceDir path:tarPath -- ) tarPack Create/overwrite a .tar / .tar.gz by packing a list of entries. Each entry is either a bare string/path (the file or directory to add, keeping its base name and mode) or a dictionary requiring path; in the dictionary form archivePath (override the in-archive name) and mode are optional. mode is a Go os.FileMode; write it with an octal literal, e.g. 0o644 (rw-r--r--), 0o755 (rwxr-xr-x), 0o600. If mode is omitted, the entry keeps the source file’s own mode. ([str|path|dict] path:tarPath -- ) tarList List archive entries as dictionaries with keys: name (string, forward-slash paths, directories end with /), compressedSize and uncompressedSize (int bytes; equal, since tar has no per-entry compressed size), isDir (bool), perm (int POSIX permission bits), executable (bool), modified (datetime), type ("file"/"dir"/"symlink"), and linkTarget (symlink target, empty otherwise). (path -- [dict]) - tarExtract Extract an entire archive. Options dict is required; defaults: overwrite=false, skipExisting=false (mutually exclusive), stripComponents=0, pattern="" (glob matched before stripping), preservePermissions=true. Destination is created if missing. (path:tarPath path:destDir dict:options -- ) - tarExtractEntry Extract a single entry (file or directory subtree) to a destination path. Options dict is required; defaults: overwrite=false, skipExisting=false (mutually exclusive), preservePermissions=true, mkdirs=true (create parent directories when needed). (path:tarPath str:entry path:dest dict:options -- ) + tarExtract Extract an entire archive. Options dict is required; defaults: overwrite=false, skipExisting=false (mutually exclusive), stripComponents=0, pattern="" (glob matched before stripping), preservePermissions=true, maxBytes=0 (0 = unlimited; a cap on the total uncompressed bytes written, guarding against decompression bombs). Destination is created if missing. (path:tarPath path:destDir dict:options -- ) + tarExtractEntry Extract a single entry (file or directory subtree) to a destination path. Options dict is required; defaults: overwrite=false, skipExisting=false (mutually exclusive), preservePermissions=true, mkdirs=true (create parent directories when needed), maxBytes=0 (0 = unlimited uncompressed-byte cap). (path:tarPath str:entry path:dest dict:options -- ) tarRead Read an entry’s bytes directly into the stack without writing to disk. Returns none when the entry does not exist. (path:tarPath str:entry -- Maybe[binary]) readFile Read a file into a string. (str -- str) readFileBytes Read a file into binary data. (str -- binary) diff --git a/doc/mshell.md b/doc/mshell.md index 492af17..fa5f0ad 100644 --- a/doc/mshell.md +++ b/doc/mshell.md @@ -1415,8 +1415,8 @@ See [Regexp.Expand](https://pkg.go.dev/regexp#Regexp.Expand) for replacement syn If `mode` is omitted, the entry keeps the source file's own mode. Type: `([str | path | {path: str | path, archivePath?: str | path, mode?: int}] str | path -- )` - `zipList`: List archive entries as dictionaries with keys: `name` (string, forward-slash paths, directories end with `/`), `compressedSize` (int bytes), `uncompressedSize` (int bytes), `isDir` (bool), `perm` (int POSIX permission bits), `executable` (bool), and `modified` (datetime from the archive entry). `(path -- [dict])` -- `zipExtract`: Extract an entire archive. Options dict is required; defaults: `overwrite=false`, `skipExisting=false` (mutually exclusive), `stripComponents=0`, `pattern=""` (glob matched before stripping), `preservePermissions=true`. Destination is created if missing. `(path:zipPath path:destDir dict:options -- )` -- `zipExtractEntry`: Extract a single entry (file or directory subtree) to a destination path. Options dict is required; defaults: `overwrite=false`, `skipExisting=false` (mutually exclusive), `preservePermissions=true`, `mkdirs=true`. `(path:zipPath str:entry path:dest dict:options -- )` +- `zipExtract`: Extract an entire archive. Options dict is required; defaults: `overwrite=false`, `skipExisting=false` (mutually exclusive), `stripComponents=0`, `pattern=""` (glob matched before stripping), `preservePermissions=true`, `maxBytes=0` (0 = unlimited; caps the total uncompressed bytes written to guard against decompression bombs). Destination is created if missing. `(path:zipPath path:destDir dict:options -- )` +- `zipExtractEntry`: Extract a single entry (file or directory subtree) to a destination path. Options dict is required; defaults: `overwrite=false`, `skipExisting=false` (mutually exclusive), `preservePermissions=true`, `mkdirs=true`, `maxBytes=0` (0 = unlimited uncompressed-byte cap). `(path:zipPath str:entry path:dest dict:options -- )` - `zipRead`: Read an entry's bytes directly onto the stack without writing to disk. Returns `none` when the entry does not exist. `(path:zipPath str:entry -- Maybe[binary])` ## Archive (Tar) Functions @@ -1431,8 +1431,14 @@ Two things differ, both driven by the tar format: read transparently regardless of its filename. - Symlinks are preserved: `tarPack`/`tarDir*` store symlinks as symlink entries, and `tarExtract`/`tarExtractEntry` recreate them (rejecting any whose target - would escape the destination directory). Hard links and device/fifo nodes are - rejected with an error. + would escape the destination directory). Extraction also refuses to write + through a symlink that already exists in the destination and points outside + it. Hard links and device/fifo nodes are rejected with an error. + +The extract functions accept an optional `maxBytes` cap (total uncompressed +bytes; `0` = unlimited) to guard against decompression bombs, and packing never +follows a source symlink, so a symlink loop or a link to `/dev/zero` cannot hang +or inflate the archive. - `tarDirInc`: Create/overwrite a `.tar`/`.tar.gz` from a directory; the archive root contains the directory's contents (no parent folder). `(path:sourceDir path:tarPath -- )` - `tarDirExc`: Create/overwrite a `.tar`/`.tar.gz` that includes the source directory itself at the archive root (entries are prefixed with the directory name). `(path:sourceDir path:tarPath -- )` @@ -1446,8 +1452,8 @@ Two things differ, both driven by the tar format: If `mode` is omitted, the entry keeps the source file's own mode. Type: `([str | path | {path: str | path, archivePath?: str | path, mode?: int}] str | path -- )` - `tarList`: List archive entries as dictionaries with keys: `name` (string, forward-slash paths, directories end with `/`), `compressedSize` and `uncompressedSize` (int bytes; equal, since tar has no per-entry compressed size), `isDir` (bool), `perm` (int POSIX permission bits), `executable` (bool), `modified` (datetime from the archive entry), `type` (`"file"`/`"dir"`/`"symlink"`), and `linkTarget` (symlink target, empty otherwise). `(path -- [dict])` -- `tarExtract`: Extract an entire archive. Options dict is required; defaults: `overwrite=false`, `skipExisting=false` (mutually exclusive), `stripComponents=0`, `pattern=""` (glob matched before stripping), `preservePermissions=true`. Destination is created if missing. `(path:tarPath path:destDir dict:options -- )` -- `tarExtractEntry`: Extract a single entry (file or directory subtree) to a destination path. Options dict is required; defaults: `overwrite=false`, `skipExisting=false` (mutually exclusive), `preservePermissions=true`, `mkdirs=true`. `(path:tarPath str:entry path:dest dict:options -- )` +- `tarExtract`: Extract an entire archive. Options dict is required; defaults: `overwrite=false`, `skipExisting=false` (mutually exclusive), `stripComponents=0`, `pattern=""` (glob matched before stripping), `preservePermissions=true`, `maxBytes=0` (0 = unlimited; caps the total uncompressed bytes written to guard against decompression bombs). Destination is created if missing. `(path:tarPath path:destDir dict:options -- )` +- `tarExtractEntry`: Extract a single entry (file or directory subtree) to a destination path. Options dict is required; defaults: `overwrite=false`, `skipExisting=false` (mutually exclusive), `preservePermissions=true`, `mkdirs=true`, `maxBytes=0` (0 = unlimited uncompressed-byte cap). `(path:tarPath str:entry path:dest dict:options -- )` - `tarRead`: Read an entry's bytes directly onto the stack without writing to disk. Returns `none` when the entry does not exist. `(path:tarPath str:entry -- Maybe[binary])` ## Variables diff --git a/mshell/Evaluator.go b/mshell/Evaluator.go index a560e04..51d81e0 100644 --- a/mshell/Evaluator.go +++ b/mshell/Evaluator.go @@ -4822,6 +4822,68 @@ type zipWriteOptions struct { overwrite bool skipExisting bool preservePermissions bool + maxBytes int64 // 0 = unlimited; total uncompressed bytes for the extraction +} + +// byteBudget bounds the total number of uncompressed bytes written during a +// single archive extraction, defending against decompression bombs. A limit of +// zero (or negative) means unlimited. +type byteBudget struct { + limit int64 + used int64 +} + +func newByteBudget(limit int64) *byteBudget { + return &byteBudget{limit: limit} +} + +// copy streams src to dst while enforcing the budget. When a limit is set it +// copies at most one byte past the remaining allowance so an overflow is +// detected deterministically, then reports which entry blew the budget. +func (b *byteBudget) copy(dst io.Writer, src io.Reader, entryName string) error { + if b == nil || b.limit <= 0 { + _, err := io.Copy(dst, src) + return err + } + remaining := b.limit - b.used + n, err := io.Copy(dst, io.LimitReader(src, remaining+1)) + b.used += n + if err != nil { + return err + } + if b.used > b.limit { + return fmt.Errorf("extraction exceeded maxBytes limit of %d bytes at entry %s", b.limit, entryName) + } + return nil +} + +// ensureRealParentWithinBase defends against writing through a symlink that +// already exists in the destination tree (for example, extracting into a +// directory that legitimately contains a symlink). It resolves the deepest +// already-existing ancestor directory of destPath, following every symlink, +// and refuses when that real location falls outside the destination root. +func ensureRealParentWithinBase(destPath, base string) error { + realBase, err := filepath.EvalSymlinks(base) + if err != nil { + return fmt.Errorf("Error resolving destination %s: %w", base, err) + } + realBaseWithSep := ensureTrailingSeparator(realBase) + + ancestor := filepath.Dir(destPath) + for { + real, err := filepath.EvalSymlinks(ancestor) + if err == nil { + return ensureWithinBase(real, realBase, realBaseWithSep) + } + if !errors.Is(err, os.ErrNotExist) { + return err + } + parent := filepath.Dir(ancestor) + if parent == ancestor { + return nil + } + ancestor = parent + } } type zipExtractOptions struct { @@ -5107,9 +5169,27 @@ func parseZipExtractOptions(dict *MShellDict) (zipExtractOptions, error) { options.pattern = val } + if err := parseMaxBytesOption(dict, &options.zipWriteOptions); err != nil { + return options, err + } + return options, nil } +// parseMaxBytesOption reads the optional `maxBytes` cap (total uncompressed +// bytes for the extraction). 0 means unlimited; negative is rejected. +func parseMaxBytesOption(dict *MShellDict, opts *zipWriteOptions) error { + if val, ok, err := intOption(dict, "maxBytes"); err != nil { + return err + } else if ok { + if val < 0 { + return fmt.Errorf("option 'maxBytes' must be >= 0") + } + opts.maxBytes = int64(val) + } + return nil +} + func parseZipExtractEntryOptions(dict *MShellDict) (zipExtractEntryOptions, error) { options := zipExtractEntryOptions{ zipWriteOptions: zipWriteOptions{ @@ -5148,6 +5228,10 @@ func parseZipExtractEntryOptions(dict *MShellDict) (zipExtractEntryOptions, erro options.mkdirs = val } + if err := parseMaxBytesOption(dict, &options.zipWriteOptions); err != nil { + return options, err + } + return options, nil } @@ -5203,6 +5287,7 @@ func extractZipArchive(zipPath, destDir string, options zipExtractOptions) error return fmt.Errorf("Error creating destination %s: %w", absDest, err) } baseWithSep := ensureTrailingSeparator(absDest) + budget := newByteBudget(options.maxBytes) for _, file := range reader.File { if file.FileInfo().Mode()&os.ModeSymlink != 0 { @@ -5238,7 +5323,7 @@ func extractZipArchive(zipPath, destDir string, options zipExtractOptions) error return err } - if err := writeZipFileToDisk(file, target, options.zipWriteOptions, true); err != nil { + if err := writeZipFileToDisk(file, target, options.zipWriteOptions, true, absDest, budget); err != nil { return err } } @@ -5321,6 +5406,7 @@ func extractZipDirectoryEntries(files []*zip.File, targetName, destPath string, baseWithSep := ensureTrailingSeparator(absDest) prefix := targetName + "/" + budget := newByteBudget(options.maxBytes) for _, file := range files { if file.FileInfo().Mode()&os.ModeSymlink != 0 { @@ -5346,7 +5432,7 @@ func extractZipDirectoryEntries(files []*zip.File, targetName, destPath string, return err } - if err := writeZipFileToDisk(file, target, options.zipWriteOptions, true); err != nil { + if err := writeZipFileToDisk(file, target, options.zipWriteOptions, true, absDest, budget); err != nil { return err } } @@ -5375,10 +5461,14 @@ func extractZipFileEntry(file *zip.File, destPath string, options zipExtractEntr } } - return writeZipFileToDisk(file, absDest, options.zipWriteOptions, false) + return writeZipFileToDisk(file, absDest, options.zipWriteOptions, false, parent, newByteBudget(options.maxBytes)) } -func writeZipFileToDisk(file *zip.File, destPath string, options zipWriteOptions, ensureParents bool) error { +func writeZipFileToDisk(file *zip.File, destPath string, options zipWriteOptions, ensureParents bool, base string, budget *byteBudget) error { + if err := ensureRealParentWithinBase(destPath, base); err != nil { + return err + } + info := file.FileInfo() if info.IsDir() { @@ -5435,7 +5525,7 @@ func writeZipFileToDisk(file *zip.File, destPath string, options zipWriteOptions } defer outFile.Close() - if _, err := io.Copy(outFile, reader); err != nil { + if err := budget.copy(outFile, reader, file.Name); err != nil { return err } diff --git a/mshell/Tar.go b/mshell/Tar.go index 0fc254b..e50f226 100644 --- a/mshell/Tar.go +++ b/mshell/Tar.go @@ -266,8 +266,16 @@ func addDirectoryToTar(tw *tar.Writer, sourcePath, archivePrefix string, modeOve // Regular files, directories, and symlinks are supported; symlinks are stored // as symlinks (preserving the target) rather than being dereferenced. func addFileToTar(tw *tar.Writer, sourcePath, entryName string, info os.FileInfo, modeOverride *os.FileMode) error { + // Reject anything that is not a regular file, directory, or symlink before + // writing a header, so we never emit a partial entry and never os.Open a + // fifo/device/socket (which could block indefinitely). + isSymlink := info.Mode()&os.ModeSymlink != 0 + if !info.IsDir() && !info.Mode().IsRegular() && !isSymlink { + return fmt.Errorf("tarPack cannot archive %s: unsupported file type", sourcePath) + } + linkTarget := "" - if info.Mode()&os.ModeSymlink != 0 { + if isSymlink { target, err := os.Readlink(sourcePath) if err != nil { return fmt.Errorf("Error reading symlink %s: %w", sourcePath, err) @@ -291,12 +299,9 @@ func addFileToTar(tw *tar.Writer, sourcePath, entryName string, info os.FileInfo return err } - if info.Mode()&os.ModeSymlink != 0 || info.IsDir() { + if isSymlink || info.IsDir() { return nil } - if !info.Mode().IsRegular() { - return fmt.Errorf("tarPack cannot archive %s: unsupported file type", sourcePath) - } file, err := os.Open(sourcePath) if err != nil { @@ -376,6 +381,7 @@ func extractTarArchive(tarPath, destDir string, options zipExtractOptions) error return fmt.Errorf("Error creating destination %s: %w", absDest, err) } baseWithSep := ensureTrailingSeparator(absDest) + budget := newByteBudget(options.maxBytes) for { header, err := reader.Next() @@ -415,7 +421,7 @@ func extractTarArchive(tarPath, destDir string, options zipExtractOptions) error return err } - if err := writeTarEntryToDisk(reader, header, target, options.zipWriteOptions, true, absDest, baseWithSep); err != nil { + if err := writeTarEntryToDisk(reader, header, target, options.zipWriteOptions, true, absDest, baseWithSep, budget); err != nil { return err } } @@ -448,6 +454,7 @@ func extractTarEntry(tarPath, entryPath, destPath string, options zipExtractEntr fileFound := false dirCreated := false baseWithSep := "" + budget := newByteBudget(options.maxBytes) for { header, err := reader.Next() @@ -462,7 +469,7 @@ func extractTarEntry(tarPath, entryPath, destPath string, options zipExtractEntr if name == targetName && !header.FileInfo().IsDir() { // Single file entry: dest is the file path. - if err := extractTarSingleFile(reader, header, absDest, options); err != nil { + if err := extractTarSingleFile(reader, header, absDest, options, budget); err != nil { return err } fileFound = true @@ -509,7 +516,7 @@ func extractTarEntry(tarPath, entryPath, destPath string, options zipExtractEntr return err } - if err := writeTarEntryToDisk(reader, header, target, options.zipWriteOptions, true, absDest, baseWithSep); err != nil { + if err := writeTarEntryToDisk(reader, header, target, options.zipWriteOptions, true, absDest, baseWithSep, budget); err != nil { return err } } @@ -520,7 +527,7 @@ func extractTarEntry(tarPath, entryPath, destPath string, options zipExtractEntr return nil } -func extractTarSingleFile(reader *tar.Reader, header *tar.Header, absDest string, options zipExtractEntryOptions) error { +func extractTarSingleFile(reader *tar.Reader, header *tar.Header, absDest string, options zipExtractEntryOptions, budget *byteBudget) error { parent := filepath.Dir(absDest) if options.mkdirs { if err := os.MkdirAll(parent, 0755); err != nil { @@ -533,13 +540,19 @@ func extractTarSingleFile(reader *tar.Reader, header *tar.Header, absDest string } baseWithSep := ensureTrailingSeparator(parent) - return writeTarEntryToDisk(reader, header, absDest, options.zipWriteOptions, false, parent, baseWithSep) + return writeTarEntryToDisk(reader, header, absDest, options.zipWriteOptions, false, parent, baseWithSep, budget) } // writeTarEntryToDisk writes one tar entry (directory, file, or symlink) to // destPath. For symlinks it validates that the link target cannot escape the -// destination root before creating it. -func writeTarEntryToDisk(reader *tar.Reader, header *tar.Header, destPath string, options zipWriteOptions, ensureParents bool, base, baseWithSep string) error { +// destination root before creating it. The archive-symlink target guard and +// the ensureRealParentWithinBase guard together stop a path component from +// redirecting a write outside the destination. +func writeTarEntryToDisk(reader *tar.Reader, header *tar.Header, destPath string, options zipWriteOptions, ensureParents bool, base, baseWithSep string, budget *byteBudget) error { + if err := ensureRealParentWithinBase(destPath, base); err != nil { + return err + } + switch header.Typeflag { case tar.TypeDir: if ensureParents { @@ -558,7 +571,7 @@ func writeTarEntryToDisk(reader *tar.Reader, header *tar.Header, destPath string case tar.TypeSymlink: return writeTarSymlink(header, destPath, options, ensureParents, base, baseWithSep) case tar.TypeReg, '\x00': // '\x00' is the legacy TypeRegA (deprecated) regular-file flag - return writeTarRegularFile(reader, header, destPath, options, ensureParents) + return writeTarRegularFile(reader, header, destPath, options, ensureParents, budget) default: return fmt.Errorf("tarExtract cannot handle entry %s (unsupported type %q)", header.Name, string(header.Typeflag)) } @@ -607,7 +620,7 @@ func writeTarSymlink(header *tar.Header, destPath string, options zipWriteOption return nil } -func writeTarRegularFile(reader *tar.Reader, header *tar.Header, destPath string, options zipWriteOptions, ensureParents bool) error { +func writeTarRegularFile(reader *tar.Reader, header *tar.Header, destPath string, options zipWriteOptions, ensureParents bool, budget *byteBudget) error { parentDir := filepath.Dir(destPath) if ensureParents { if err := os.MkdirAll(parentDir, 0755); err != nil { @@ -640,7 +653,7 @@ func writeTarRegularFile(reader *tar.Reader, header *tar.Header, destPath string } defer outFile.Close() - if _, err := io.Copy(outFile, reader); err != nil { + if err := budget.copy(outFile, reader, header.Name); err != nil { return err } diff --git a/mshell/Tar_test.go b/mshell/Tar_test.go new file mode 100644 index 0000000..991faa7 --- /dev/null +++ b/mshell/Tar_test.go @@ -0,0 +1,262 @@ +package main + +import ( + "archive/tar" + "compress/gzip" + "os" + "path/filepath" + "strings" + "testing" +) + +// writeTarArchive builds a tar (optionally gzip-compressed) at path using the +// supplied callback to add entries. +func writeTarArchive(t *testing.T, path string, gzipped bool, add func(tw *tar.Writer)) { + t.Helper() + f, err := os.Create(path) + if err != nil { + t.Fatalf("create %s: %v", path, err) + } + defer f.Close() + + var tw *tar.Writer + if gzipped { + gz := gzip.NewWriter(f) + tw = tar.NewWriter(gz) + add(tw) + if err := tw.Close(); err != nil { + t.Fatalf("close tar: %v", err) + } + if err := gz.Close(); err != nil { + t.Fatalf("close gzip: %v", err) + } + return + } + tw = tar.NewWriter(f) + add(tw) + if err := tw.Close(); err != nil { + t.Fatalf("close tar: %v", err) + } +} + +func addTarFile(t *testing.T, tw *tar.Writer, name, content string) { + t.Helper() + hdr := &tar.Header{Name: name, Mode: 0o644, Size: int64(len(content)), Typeflag: tar.TypeReg} + if err := tw.WriteHeader(hdr); err != nil { + t.Fatalf("write header %s: %v", name, err) + } + if _, err := tw.Write([]byte(content)); err != nil { + t.Fatalf("write body %s: %v", name, err) + } +} + +func addTarSymlink(t *testing.T, tw *tar.Writer, name, target string) { + t.Helper() + hdr := &tar.Header{Name: name, Mode: 0o777, Typeflag: tar.TypeSymlink, Linkname: target} + if err := tw.WriteHeader(hdr); err != nil { + t.Fatalf("write symlink header %s: %v", name, err) + } +} + +func extractAll(tarPath, dest string) error { + opts := zipExtractOptions{zipWriteOptions: zipWriteOptions{overwrite: true, preservePermissions: true}} + return extractTarArchive(tarPath, dest, opts) +} + +func TestTarRoundTripAndGzipAutodetect(t *testing.T) { + dir := t.TempDir() + src := filepath.Join(dir, "src") + if err := os.MkdirAll(filepath.Join(src, "sub"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(src, "a.txt"), []byte("hello"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(src, "sub", "b.txt"), []byte("world"), 0o644); err != nil { + t.Fatal(err) + } + + for _, name := range []string{"out.tar", "out.tar.gz"} { + archive := filepath.Join(dir, name) + if err := buildTarFromEntries([]zipPackItem{{SourcePath: src, PreserveRoot: true}}, archive); err != nil { + t.Fatalf("%s pack: %v", name, err) + } + entries, err := collectTarMetadata(archive) + if err != nil { + t.Fatalf("%s list: %v", name, err) + } + found := map[string]bool{} + for _, e := range entries { + found[e.Name] = true + } + for _, want := range []string{"src/a.txt", "src/sub/b.txt"} { + if !found[want] { + t.Errorf("%s: missing entry %s (got %v)", name, want, found) + } + } + data, ok, err := readTarEntry(archive, "src/a.txt") + if err != nil || !ok || string(data) != "hello" { + t.Errorf("%s: readTarEntry a.txt = %q ok=%v err=%v", name, data, ok, err) + } + } + + // Auto-detect: a gzip tarball with a non-gzip name must still be read via + // magic-byte sniffing. + mystery := filepath.Join(dir, "mystery") + if err := os.Rename(filepath.Join(dir, "out.tar.gz"), mystery); err != nil { + t.Fatal(err) + } + if _, err := collectTarMetadata(mystery); err != nil { + t.Errorf("auto-detect list of renamed gzip failed: %v", err) + } +} + +func TestTarExtractRejectsPathTraversal(t *testing.T) { + dir := t.TempDir() + archive := filepath.Join(dir, "trav.tar") + writeTarArchive(t, archive, false, func(tw *tar.Writer) { + addTarFile(t, tw, "../escape.txt", "pwned") + }) + dest := filepath.Join(dir, "dest") + if err := extractAll(archive, dest); err == nil { + t.Fatal("expected path-traversal extraction to be refused") + } + if _, err := os.Stat(filepath.Join(dir, "escape.txt")); err == nil { + t.Fatal("path traversal escaped the destination directory") + } +} + +func TestTarExtractRejectsEscapingSymlink(t *testing.T) { + dir := t.TempDir() + outside := filepath.Join(dir, "outside") + archive := filepath.Join(dir, "sym.tar") + writeTarArchive(t, archive, false, func(tw *tar.Writer) { + addTarSymlink(t, tw, "bad", outside) + }) + dest := filepath.Join(dir, "dest") + if err := extractAll(archive, dest); err == nil { + t.Fatal("expected escaping symlink extraction to be refused") + } +} + +func TestTarExtractRejectsHardlink(t *testing.T) { + dir := t.TempDir() + archive := filepath.Join(dir, "hl.tar") + writeTarArchive(t, archive, false, func(tw *tar.Writer) { + hdr := &tar.Header{Name: "hl", Typeflag: tar.TypeLink, Linkname: "/etc/passwd", Mode: 0o644} + if err := tw.WriteHeader(hdr); err != nil { + t.Fatal(err) + } + }) + dest := filepath.Join(dir, "dest") + if err := extractAll(archive, dest); err == nil { + t.Fatal("expected hardlink entry to be rejected") + } +} + +func TestTarExtractRejectsWriteThroughPreexistingSymlink(t *testing.T) { + dir := t.TempDir() + outside := filepath.Join(dir, "outside") + if err := os.MkdirAll(outside, 0o755); err != nil { + t.Fatal(err) + } + dest := filepath.Join(dir, "dest") + if err := os.MkdirAll(dest, 0o755); err != nil { + t.Fatal(err) + } + // Pre-existing symlink inside the destination that points outside it. + if err := os.Symlink(outside, filepath.Join(dest, "link")); err != nil { + t.Skipf("cannot create symlink: %v", err) + } + + archive := filepath.Join(dir, "wt.tar") + writeTarArchive(t, archive, false, func(tw *tar.Writer) { + addTarFile(t, tw, "link/pwned.txt", "pwned") + }) + if err := extractAll(archive, dest); err == nil { + t.Fatal("expected write-through pre-existing symlink to be refused") + } + if _, err := os.Stat(filepath.Join(outside, "pwned.txt")); err == nil { + t.Fatal("write escaped through pre-existing symlink") + } +} + +func TestTarExtractAllowsInDestSymlink(t *testing.T) { + dir := t.TempDir() + dest := filepath.Join(dir, "dest") + if err := os.MkdirAll(filepath.Join(dest, "realdir"), 0o755); err != nil { + t.Fatal(err) + } + // A symlink that stays within the destination must remain usable. + if err := os.Symlink("realdir", filepath.Join(dest, "s")); err != nil { + t.Skipf("cannot create symlink: %v", err) + } + + archive := filepath.Join(dir, "ok.tar") + writeTarArchive(t, archive, false, func(tw *tar.Writer) { + addTarFile(t, tw, "s/f.txt", "ok") + }) + if err := extractAll(archive, dest); err != nil { + t.Fatalf("in-dest symlink write should be allowed: %v", err) + } + got, err := os.ReadFile(filepath.Join(dest, "realdir", "f.txt")) + if err != nil || string(got) != "ok" { + t.Fatalf("expected file written through in-dest symlink, got %q err=%v", got, err) + } +} + +func TestTarExtractStripComponentsAndPattern(t *testing.T) { + dir := t.TempDir() + archive := filepath.Join(dir, "s.tar") + writeTarArchive(t, archive, false, func(tw *tar.Writer) { + addTarFile(t, tw, "top/keep.txt", "k") + addTarFile(t, tw, "top/skip.log", "s") + }) + + dest := filepath.Join(dir, "dest") + opts := zipExtractOptions{ + zipWriteOptions: zipWriteOptions{overwrite: true, preservePermissions: true}, + stripComponents: 1, + pattern: "top/*.txt", + } + if err := extractTarArchive(archive, dest, opts); err != nil { + t.Fatalf("extract: %v", err) + } + if _, err := os.Stat(filepath.Join(dest, "keep.txt")); err != nil { + t.Errorf("expected keep.txt after strip+pattern: %v", err) + } + if _, err := os.Stat(filepath.Join(dest, "skip.log")); !strings.Contains(errString(err), "no such") && err == nil { + t.Errorf("skip.log should have been filtered out by pattern") + } +} + +func errString(err error) string { + if err == nil { + return "" + } + return err.Error() +} + +func TestTarExtractMaxBytesCapsBomb(t *testing.T) { + dir := t.TempDir() + archive := filepath.Join(dir, "big.tar") + big := strings.Repeat("A", 5_000_000) + writeTarArchive(t, archive, false, func(tw *tar.Writer) { + addTarFile(t, tw, "big.bin", big) + }) + dest := filepath.Join(dir, "dest") + + opts := zipExtractOptions{zipWriteOptions: zipWriteOptions{overwrite: true, preservePermissions: true, maxBytes: 1_000_000}} + if err := extractTarArchive(archive, dest, opts); err == nil { + t.Fatal("expected extraction to exceed maxBytes and fail") + } + + // Under the cap it must succeed. + small := filepath.Join(dir, "small.tar") + writeTarArchive(t, small, false, func(tw *tar.Writer) { + addTarFile(t, tw, "ok.txt", "hello") + }) + if err := extractTarArchive(small, filepath.Join(dir, "d2"), opts); err != nil { + t.Fatalf("small archive under cap should succeed: %v", err) + } +} diff --git a/mshell/Tar_unix_test.go b/mshell/Tar_unix_test.go new file mode 100644 index 0000000..a0ee107 --- /dev/null +++ b/mshell/Tar_unix_test.go @@ -0,0 +1,43 @@ +//go:build !windows + +package main + +import ( + "os" + "path/filepath" + "syscall" + "testing" + "time" +) + +// TestTarPackRejectsFifoWithoutHanging verifies that packing a directory +// containing a FIFO fails promptly with an "unsupported file type" error +// instead of blocking forever on os.Open of the pipe (which has no writer). +func TestTarPackRejectsFifoWithoutHanging(t *testing.T) { + dir := t.TempDir() + src := filepath.Join(dir, "src") + if err := os.MkdirAll(src, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(src, "real.txt"), []byte("ok"), 0o644); err != nil { + t.Fatal(err) + } + if err := syscall.Mkfifo(filepath.Join(src, "pipe"), 0o644); err != nil { + t.Skipf("cannot create fifo: %v", err) + } + + archive := filepath.Join(dir, "out.tar") + done := make(chan error, 1) + go func() { + done <- buildTarFromEntries([]zipPackItem{{SourcePath: src, PreserveRoot: true}}, archive) + }() + + select { + case err := <-done: + if err == nil { + t.Fatal("expected an error packing a directory containing a FIFO") + } + case <-time.After(10 * time.Second): + t.Fatal("buildTarFromEntries hung on a FIFO source entry") + } +} diff --git a/mshell/TypeBuiltins.go b/mshell/TypeBuiltins.go index 6ab1b41..1523fbd 100644 --- a/mshell/TypeBuiltins.go +++ b/mshell/TypeBuiltins.go @@ -594,8 +594,8 @@ func builtinSigsByName(arena *TypeArena, names *NameTable) map[NameId][]QuoteSig r.reg("zipPack", "([str | path | {path: str | path, archivePath?: str | path, mode?: int}] str | path -- )") // zipExtract / zipExtractEntry options are all optional; the dict is // required positionally but may be empty. - zipExtractOpts := "{overwrite?: bool, skipExisting?: bool, stripComponents?: int, pattern?: str, preservePermissions?: bool}" - zipEntryOpts := "{overwrite?: bool, skipExisting?: bool, preservePermissions?: bool, mkdirs?: bool}" + zipExtractOpts := "{overwrite?: bool, skipExisting?: bool, stripComponents?: int, pattern?: str, preservePermissions?: bool, maxBytes?: int}" + zipEntryOpts := "{overwrite?: bool, skipExisting?: bool, preservePermissions?: bool, mkdirs?: bool, maxBytes?: int}" r.reg("zipExtract", "(str | path str | path "+zipExtractOpts+" -- )") r.reg("zipExtractEntry", "(str str str "+zipEntryOpts+" -- )", diff --git a/mshell/ZipHardening_test.go b/mshell/ZipHardening_test.go new file mode 100644 index 0000000..f2aab34 --- /dev/null +++ b/mshell/ZipHardening_test.go @@ -0,0 +1,79 @@ +package main + +import ( + "archive/zip" + "os" + "path/filepath" + "strings" + "testing" +) + +// writeZipArchive builds a zip at path using the supplied callback. +func writeZipArchive(t *testing.T, path string, add func(zw *zip.Writer)) { + t.Helper() + f, err := os.Create(path) + if err != nil { + t.Fatalf("create %s: %v", path, err) + } + defer f.Close() + zw := zip.NewWriter(f) + add(zw) + if err := zw.Close(); err != nil { + t.Fatalf("close zip: %v", err) + } +} + +// TestZipExtractRejectsWriteThroughPreexistingSymlink verifies the shared +// ensureRealParentWithinBase guard now protects the zip extractor too. +func TestZipExtractRejectsWriteThroughPreexistingSymlink(t *testing.T) { + dir := t.TempDir() + outside := filepath.Join(dir, "outside") + if err := os.MkdirAll(outside, 0o755); err != nil { + t.Fatal(err) + } + dest := filepath.Join(dir, "dest") + if err := os.MkdirAll(dest, 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(outside, filepath.Join(dest, "link")); err != nil { + t.Skipf("cannot create symlink: %v", err) + } + + archive := filepath.Join(dir, "wt.zip") + writeZipArchive(t, archive, func(zw *zip.Writer) { + w, err := zw.Create("link/pwned.txt") + if err != nil { + t.Fatal(err) + } + if _, err := w.Write([]byte("pwned")); err != nil { + t.Fatal(err) + } + }) + + opts := zipExtractOptions{zipWriteOptions: zipWriteOptions{overwrite: true, preservePermissions: true}} + if err := extractZipArchive(archive, dest, opts); err == nil { + t.Fatal("expected write-through pre-existing symlink to be refused") + } + if _, err := os.Stat(filepath.Join(outside, "pwned.txt")); err == nil { + t.Fatal("zip write escaped through pre-existing symlink") + } +} + +func TestZipExtractMaxBytesCapsBomb(t *testing.T) { + dir := t.TempDir() + archive := filepath.Join(dir, "big.zip") + writeZipArchive(t, archive, func(zw *zip.Writer) { + w, err := zw.Create("big.bin") + if err != nil { + t.Fatal(err) + } + if _, err := w.Write([]byte(strings.Repeat("A", 5_000_000))); err != nil { + t.Fatal(err) + } + }) + + opts := zipExtractOptions{zipWriteOptions: zipWriteOptions{overwrite: true, preservePermissions: true, maxBytes: 1_000_000}} + if err := extractZipArchive(archive, filepath.Join(dir, "dest"), opts); err == nil { + t.Fatal("expected zip extraction to exceed maxBytes and fail") + } +} From 19bbf1867984fcdd33b27ab68e9e438f6a107f63 Mon Sep 17 00:00:00 2001 From: Mitchell Paulus Date: Tue, 7 Jul 2026 10:37:54 -0500 Subject: [PATCH 3/5] Don't follow a symlink at the final path component on extraction Reviewed GNU tar's extract.c and its security history for hardening we lack. Its open_output_file / maybe_recoverable logic never writes through a symlink at the target name: it creates with O_EXCL by default and, when overwriting, uses O_NOFOLLOW or unlinks the existing name before recreating. We previously created regular files with O_CREATE|O_WRONLY|O_TRUNC after a stat()-based existence check. Our pre-existing-symlink guard only resolved the parent directory, so with overwrite enabled, extracting an entry whose destination name was already a symlink to an outside file wrote THROUGH it and clobbered the target (verified: dest/victim -> ../secret was overwritten). Route both the tar and zip regular-file writers through createExtractedFile, which opens O_CREATE|O_WRONLY|O_EXCL (never follows a symlink, even a dangling one), and for overwrite removes the existing name itself (os.Remove unlinks the link, not its target) then creates a fresh file atomically. skipExisting and default-error semantics are preserved. Adds regression tests for tar and zip. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 6 ++++ mshell/Evaluator.go | 64 ++++++++++++++++++++++++++----------- mshell/Tar.go | 19 +++-------- mshell/Tar_test.go | 37 +++++++++++++++++++++ mshell/ZipHardening_test.go | 36 +++++++++++++++++++++ 5 files changed, 129 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bc8650..fd8b4cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 symlink that already exists in the destination directory and points outside it, closing a path-traversal vector when extracting into a directory that contains symlinks. +- Archive extraction (both `zip*` and `tar*`) no longer follows a symlink at + the final path component: regular files are created with `O_EXCL`, and in + `overwrite` mode an existing name is unlinked (never dereferenced) before a + fresh file is created. This mirrors GNU tar's behavior and prevents an + `overwrite` extraction from writing through a pre-existing symlink at the + destination name (e.g. `dest/report` -> `/etc/passwd`). - Optional fields in dictionary shape types, written `name?: T` (and `"name"?: T` in `def` signatures). An optional field may be absent from a value; when present, its value is still type-checked. This lets option-style diff --git a/mshell/Evaluator.go b/mshell/Evaluator.go index 51d81e0..5adfe90 100644 --- a/mshell/Evaluator.go +++ b/mshell/Evaluator.go @@ -4857,6 +4857,45 @@ func (b *byteBudget) copy(dst io.Writer, src io.Reader, entryName string) error return nil } +// createExtractedFile opens the destination for a regular-file entry without +// ever following a symlink at the final path component. It mirrors GNU tar's +// open_output_file / maybe_recoverable logic: O_EXCL means an existing name +// (regular file, symlink — even dangling — or directory) never gets written +// through. On collision it honors the skip/overwrite options, and for +// overwrite it removes the existing name itself (os.Remove unlinks a symlink +// rather than following it) before creating a fresh file. Returns (nil, nil) +// when skipExisting applies and the entry should be skipped. +func createExtractedFile(destPath string, mode os.FileMode, options zipWriteOptions) (*os.File, error) { + flags := os.O_CREATE | os.O_WRONLY | os.O_EXCL + f, err := os.OpenFile(destPath, flags, mode) + if err == nil { + return f, nil + } + if !errors.Is(err, os.ErrExist) { + return nil, err + } + + if options.skipExisting { + return nil, nil + } + if !options.overwrite { + return nil, fmt.Errorf("Destination %s already exists", destPath) + } + + // Overwrite: unlink the existing name (not its symlink target), then + // create a fresh regular file. O_EXCL keeps the recreate atomic, so a + // symlink slipped back in after the remove causes a clean failure rather + // than a write-through. + if err := os.Remove(destPath); err != nil { + return nil, fmt.Errorf("Error replacing %s: %w", destPath, err) + } + f, err = os.OpenFile(destPath, flags, mode) + if err != nil { + return nil, err + } + return f, nil +} + // ensureRealParentWithinBase defends against writing through a symlink that // already exists in the destination tree (for example, extracting into a // directory that legitimately contains a symlink). It resolves the deepest @@ -5499,31 +5538,20 @@ func writeZipFileToDisk(file *zip.File, destPath string, options zipWriteOptions } } - if options.skipExisting { - if _, err := os.Stat(destPath); err == nil { - return nil - } else if !errors.Is(err, os.ErrNotExist) { - return err - } - } else { - if _, err := os.Stat(destPath); err == nil && !options.overwrite { - return fmt.Errorf("Destination %s already exists", destPath) - } else if err != nil && !errors.Is(err, os.ErrNotExist) { - return err - } - } - - reader, err := file.Open() + outFile, err := createExtractedFile(destPath, info.Mode().Perm(), options) if err != nil { return err } - defer reader.Close() + if outFile == nil { + return nil // skipExisting: the destination already exists + } + defer outFile.Close() - outFile, err := os.OpenFile(destPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, info.Mode().Perm()) + reader, err := file.Open() if err != nil { return err } - defer outFile.Close() + defer reader.Close() if err := budget.copy(outFile, reader, file.Name); err != nil { return err diff --git a/mshell/Tar.go b/mshell/Tar.go index e50f226..0763478 100644 --- a/mshell/Tar.go +++ b/mshell/Tar.go @@ -632,25 +632,14 @@ func writeTarRegularFile(reader *tar.Reader, header *tar.Header, destPath string } } - if options.skipExisting { - if _, err := os.Stat(destPath); err == nil { - return nil - } else if !errors.Is(err, os.ErrNotExist) { - return err - } - } else { - if _, err := os.Stat(destPath); err == nil && !options.overwrite { - return fmt.Errorf("Destination %s already exists", destPath) - } else if err != nil && !errors.Is(err, os.ErrNotExist) { - return err - } - } - mode := header.FileInfo().Mode().Perm() - outFile, err := os.OpenFile(destPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, mode) + outFile, err := createExtractedFile(destPath, mode, options) if err != nil { return err } + if outFile == nil { + return nil // skipExisting: the destination already exists + } defer outFile.Close() if err := budget.copy(outFile, reader, header.Name); err != nil { diff --git a/mshell/Tar_test.go b/mshell/Tar_test.go index 991faa7..0975caf 100644 --- a/mshell/Tar_test.go +++ b/mshell/Tar_test.go @@ -237,6 +237,43 @@ func errString(err error) string { return err.Error() } +// TestTarExtractOverwriteDoesNotFollowFinalSymlink checks that extracting with +// overwrite over a destination name that is already a symlink to an outside +// file replaces the symlink with a fresh file rather than writing through it. +func TestTarExtractOverwriteDoesNotFollowFinalSymlink(t *testing.T) { + dir := t.TempDir() + outside := filepath.Join(dir, "outside.txt") + if err := os.WriteFile(outside, []byte("ORIGINAL"), 0o644); err != nil { + t.Fatal(err) + } + dest := filepath.Join(dir, "dest") + if err := os.MkdirAll(dest, 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(outside, filepath.Join(dest, "victim")); err != nil { + t.Skipf("cannot create symlink: %v", err) + } + + archive := filepath.Join(dir, "v.tar") + writeTarArchive(t, archive, false, func(tw *tar.Writer) { + addTarFile(t, tw, "victim", "REPLACED") + }) + opts := zipExtractOptions{zipWriteOptions: zipWriteOptions{overwrite: true, preservePermissions: true}} + if err := extractTarArchive(archive, dest, opts); err != nil { + t.Fatalf("extract: %v", err) + } + if got, _ := os.ReadFile(outside); string(got) != "ORIGINAL" { + t.Fatalf("wrote THROUGH the symlink: outside file is now %q", got) + } + fi, err := os.Lstat(filepath.Join(dest, "victim")) + if err != nil || fi.Mode()&os.ModeSymlink != 0 { + t.Fatalf("dest/victim should be a fresh regular file, got mode %v err %v", fi.Mode(), err) + } + if got, _ := os.ReadFile(filepath.Join(dest, "victim")); string(got) != "REPLACED" { + t.Fatalf("dest/victim content = %q, want REPLACED", got) + } +} + func TestTarExtractMaxBytesCapsBomb(t *testing.T) { dir := t.TempDir() archive := filepath.Join(dir, "big.tar") diff --git a/mshell/ZipHardening_test.go b/mshell/ZipHardening_test.go index f2aab34..b0b91ad 100644 --- a/mshell/ZipHardening_test.go +++ b/mshell/ZipHardening_test.go @@ -59,6 +59,42 @@ func TestZipExtractRejectsWriteThroughPreexistingSymlink(t *testing.T) { } } +// TestZipExtractOverwriteDoesNotFollowFinalSymlink is the zip analog of the +// tar test: overwriting a destination name that is a symlink must not write +// through it. +func TestZipExtractOverwriteDoesNotFollowFinalSymlink(t *testing.T) { + dir := t.TempDir() + outside := filepath.Join(dir, "outside.txt") + if err := os.WriteFile(outside, []byte("ORIGINAL"), 0o644); err != nil { + t.Fatal(err) + } + dest := filepath.Join(dir, "dest") + if err := os.MkdirAll(dest, 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(outside, filepath.Join(dest, "victim")); err != nil { + t.Skipf("cannot create symlink: %v", err) + } + + archive := filepath.Join(dir, "v.zip") + writeZipArchive(t, archive, func(zw *zip.Writer) { + w, err := zw.Create("victim") + if err != nil { + t.Fatal(err) + } + if _, err := w.Write([]byte("REPLACED")); err != nil { + t.Fatal(err) + } + }) + opts := zipExtractOptions{zipWriteOptions: zipWriteOptions{overwrite: true, preservePermissions: true}} + if err := extractZipArchive(archive, dest, opts); err != nil { + t.Fatalf("extract: %v", err) + } + if got, _ := os.ReadFile(outside); string(got) != "ORIGINAL" { + t.Fatalf("zip wrote THROUGH the symlink: outside file is now %q", got) + } +} + func TestZipExtractMaxBytesCapsBomb(t *testing.T) { dir := t.TempDir() archive := filepath.Join(dir, "big.zip") From 74304005d9d16b2e819158aceaf783934d1e7a60 Mon Sep 17 00:00:00 2001 From: Mitchell Paulus Date: Tue, 7 Jul 2026 10:43:58 -0500 Subject: [PATCH 4/5] Extraction: fd-based chmod, NUL-path guard, link-target tests Smaller GNU-tar-derived hardenings for the extraction path: - Restore permissions via the open file descriptor (outFile.Chmod) instead of os.Chmod(path), so a metadata restore can never be redirected through a symlink at the destination name. - Reject an archive entry name or symlink target containing an embedded NUL byte before it is used as a path (belt-and-suspenders: Go's archive/tar refuses to encode a NUL, and the OS rejects NUL paths, but a hand-crafted PAX record could carry one). - Add regression tests for escaping symlink targets (absolute and ../ forms) and a direct test of the NUL guard. Co-Authored-By: Claude Opus 4.8 (1M context) --- mshell/Evaluator.go | 21 ++++++++++++++++++++- mshell/Tar.go | 7 ++++++- mshell/Tar_test.go | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 2 deletions(-) diff --git a/mshell/Evaluator.go b/mshell/Evaluator.go index 5adfe90..5b7fc73 100644 --- a/mshell/Evaluator.go +++ b/mshell/Evaluator.go @@ -4896,6 +4896,20 @@ func createExtractedFile(destPath string, mode os.FileMode, options zipWriteOpti return f, nil } +// rejectNULInPath refuses an archive entry name or link target that contains +// an embedded NUL byte before it is used as a filesystem path. The OS would +// reject it too, but rejecting early keeps the failure clear and avoids +// relying on that behavior. (A crafted PAX path/linkpath record can carry a +// NUL that Go's archive/tar does not necessarily strip.) +func rejectNULInPath(fields ...string) error { + for _, f := range fields { + if strings.IndexByte(f, 0) >= 0 { + return fmt.Errorf("Refusing entry with embedded NUL byte in path %q", f) + } + } + return nil +} + // ensureRealParentWithinBase defends against writing through a symlink that // already exists in the destination tree (for example, extracting into a // directory that legitimately contains a symlink). It resolves the deepest @@ -5504,6 +5518,9 @@ func extractZipFileEntry(file *zip.File, destPath string, options zipExtractEntr } func writeZipFileToDisk(file *zip.File, destPath string, options zipWriteOptions, ensureParents bool, base string, budget *byteBudget) error { + if err := rejectNULInPath(file.Name); err != nil { + return err + } if err := ensureRealParentWithinBase(destPath, base); err != nil { return err } @@ -5558,7 +5575,9 @@ func writeZipFileToDisk(file *zip.File, destPath string, options zipWriteOptions } if options.preservePermissions { - if err := os.Chmod(destPath, info.Mode()); err != nil && !errors.Is(err, os.ErrPermission) { + // Chmod via the open descriptor, not the path, so permissions can + // never be redirected onto a symlink target. + if err := outFile.Chmod(info.Mode()); err != nil && !errors.Is(err, os.ErrPermission) { return fmt.Errorf("Error setting permissions on %s: %w", destPath, err) } } diff --git a/mshell/Tar.go b/mshell/Tar.go index 0763478..70ecc7f 100644 --- a/mshell/Tar.go +++ b/mshell/Tar.go @@ -549,6 +549,9 @@ func extractTarSingleFile(reader *tar.Reader, header *tar.Header, absDest string // the ensureRealParentWithinBase guard together stop a path component from // redirecting a write outside the destination. func writeTarEntryToDisk(reader *tar.Reader, header *tar.Header, destPath string, options zipWriteOptions, ensureParents bool, base, baseWithSep string, budget *byteBudget) error { + if err := rejectNULInPath(header.Name, header.Linkname); err != nil { + return err + } if err := ensureRealParentWithinBase(destPath, base); err != nil { return err } @@ -647,7 +650,9 @@ func writeTarRegularFile(reader *tar.Reader, header *tar.Header, destPath string } if options.preservePermissions { - if err := os.Chmod(destPath, mode); err != nil && !errors.Is(err, os.ErrPermission) { + // Chmod via the open descriptor, not the path, so permissions can + // never be redirected onto a symlink target. + if err := outFile.Chmod(mode); err != nil && !errors.Is(err, os.ErrPermission) { return fmt.Errorf("Error setting permissions on %s: %w", destPath, err) } } diff --git a/mshell/Tar_test.go b/mshell/Tar_test.go index 0975caf..83878a8 100644 --- a/mshell/Tar_test.go +++ b/mshell/Tar_test.go @@ -139,6 +139,43 @@ func TestTarExtractRejectsEscapingSymlink(t *testing.T) { } } +// TestTarExtractRejectsSymlinkTargets covers both flavors of an escaping +// symlink target: an absolute path and a relative "../" path. +func TestTarExtractRejectsSymlinkTargets(t *testing.T) { + cases := map[string]string{ + "absolute": "/etc/passwd", + "relative": "../../../../etc/passwd", + } + for name, target := range cases { + t.Run(name, func(t *testing.T) { + dir := t.TempDir() + archive := filepath.Join(dir, "s.tar") + writeTarArchive(t, archive, false, func(tw *tar.Writer) { + addTarSymlink(t, tw, "link", target) + }) + if err := extractAll(archive, filepath.Join(dir, "dest")); err == nil { + t.Fatalf("expected symlink with escaping target %q to be refused", target) + } + }) + } +} + +// TestRejectNULInPath verifies the NUL-byte guard directly. (Go's own +// archive/tar refuses to encode a NUL in a name, so such an entry cannot be +// produced through its writer; the guard defends against a hand-crafted PAX +// record whose value survives decoding.) +func TestRejectNULInPath(t *testing.T) { + if err := rejectNULInPath("ok/name.txt", "also/fine"); err != nil { + t.Fatalf("clean paths should pass: %v", err) + } + if err := rejectNULInPath("bad\x00name.txt"); err == nil { + t.Fatal("expected embedded NUL in name to be rejected") + } + if err := rejectNULInPath("fine", "link\x00target"); err == nil { + t.Fatal("expected embedded NUL in link target to be rejected") + } +} + func TestTarExtractRejectsHardlink(t *testing.T) { dir := t.TempDir() archive := filepath.Join(dir, "hl.tar") From 1f5b22a952abb7e4d4c9f366fb90c56390774925 Mon Sep 17 00:00:00 2001 From: Mitchell Paulus Date: Tue, 7 Jul 2026 10:56:38 -0500 Subject: [PATCH 5/5] Extract through os.Root for kernel-enforced containment GNU tar's decade of symlink-race fixes concluded that lexical/at-check-time containment is inherently TOCTOU-prone, and it now opens every extraction path via openat2(RESOLVE_BENEATH) so the kernel refuses escapes atomically (CVE-2025-45582). Go 1.24+ exposes the same mechanism as os.Root. Route all tar and zip extraction writes (extract-all, single-entry, and subtree) through an os.Root anchored at the destination. Every Mkdir/OpenFile/ Symlink/Chmod/Remove is resolved relative to the root fd, so the kernel blocks any path that escapes via ".." or a symlink component, race-free, while still permitting symlinks that stay within the destination (verified). The prior lexical normalization + containment check is kept as a cheap first layer, and regular files are still created with O_EXCL (no write-through at the final component). Removes the now-superseded ensureRealParentWithinBase EvalSymlinks guard and the non-root createExtractedFile. Behavior is unchanged for valid archives; the full test suite and the adversarial cases (traversal, escaping and pre-existing symlinks, hardlinks, final-component symlink overwrite, in-dest symlink) all pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 6 ++ mshell/Evaluator.go | 158 ++++++++++++++++------------------------ mshell/Tar.go | 174 ++++++++++++++++++++++++++------------------ 3 files changed, 171 insertions(+), 167 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd8b4cb..e686fc3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 fresh file is created. This mirrors GNU tar's behavior and prevents an `overwrite` extraction from writing through a pre-existing symlink at the destination name (e.g. `dest/report` -> `/etc/passwd`). +- Archive extraction (both `zip*` and `tar*`) now performs every write through + an `os.Root` anchored at the destination directory. The kernel enforces that + no path can escape the destination via `..` or a symlink component (using + `openat2`/`RESOLVE_BENEATH` on Linux), closing the time-of-check/time-of-use + race that a purely lexical containment check leaves open. Legitimate symlinks + that stay within the destination continue to work. - Optional fields in dictionary shape types, written `name?: T` (and `"name"?: T` in `def` signatures). An optional field may be absent from a value; when present, its value is still type-checked. This lets option-style diff --git a/mshell/Evaluator.go b/mshell/Evaluator.go index 5b7fc73..ed81979 100644 --- a/mshell/Evaluator.go +++ b/mshell/Evaluator.go @@ -4857,17 +4857,28 @@ func (b *byteBudget) copy(dst io.Writer, src io.Reader, entryName string) error return nil } -// createExtractedFile opens the destination for a regular-file entry without -// ever following a symlink at the final path component. It mirrors GNU tar's -// open_output_file / maybe_recoverable logic: O_EXCL means an existing name -// (regular file, symlink — even dangling — or directory) never gets written -// through. On collision it honors the skip/overwrite options, and for -// overwrite it removes the existing name itself (os.Remove unlinks a symlink -// rather than following it) before creating a fresh file. Returns (nil, nil) -// when skipExisting applies and the entry should be skipped. -func createExtractedFile(destPath string, mode os.FileMode, options zipWriteOptions) (*os.File, error) { +// rejectNULInPath refuses an archive entry name or link target that contains +// an embedded NUL byte before it is used as a filesystem path. The OS would +// reject it too, but rejecting early keeps the failure clear and avoids +// relying on that behavior. (A crafted PAX path/linkpath record can carry a +// NUL that Go's archive/tar does not necessarily strip.) +func rejectNULInPath(fields ...string) error { + for _, f := range fields { + if strings.IndexByte(f, 0) >= 0 { + return fmt.Errorf("Refusing entry with embedded NUL byte in path %q", f) + } + } + return nil +} + +// createExtractedFileInRoot is the os.Root-relative form of createExtractedFile: +// it opens rel (a root-relative path) with O_EXCL so an existing name is never +// written through, and for overwrite removes the existing name (via root, which +// cannot follow an escaping symlink) before an exclusive recreate. Returns +// (nil, nil) when skipExisting applies. display is only for error messages. +func createExtractedFileInRoot(root *os.Root, rel string, mode os.FileMode, options zipWriteOptions, display string) (*os.File, error) { flags := os.O_CREATE | os.O_WRONLY | os.O_EXCL - f, err := os.OpenFile(destPath, flags, mode) + f, err := root.OpenFile(rel, flags, mode) if err == nil { return f, nil } @@ -4879,66 +4890,19 @@ func createExtractedFile(destPath string, mode os.FileMode, options zipWriteOpti return nil, nil } if !options.overwrite { - return nil, fmt.Errorf("Destination %s already exists", destPath) + return nil, fmt.Errorf("Destination %s already exists", display) } - // Overwrite: unlink the existing name (not its symlink target), then - // create a fresh regular file. O_EXCL keeps the recreate atomic, so a - // symlink slipped back in after the remove causes a clean failure rather - // than a write-through. - if err := os.Remove(destPath); err != nil { - return nil, fmt.Errorf("Error replacing %s: %w", destPath, err) + if err := root.Remove(rel); err != nil { + return nil, fmt.Errorf("Error replacing %s: %w", display, err) } - f, err = os.OpenFile(destPath, flags, mode) + f, err = root.OpenFile(rel, flags, mode) if err != nil { return nil, err } return f, nil } -// rejectNULInPath refuses an archive entry name or link target that contains -// an embedded NUL byte before it is used as a filesystem path. The OS would -// reject it too, but rejecting early keeps the failure clear and avoids -// relying on that behavior. (A crafted PAX path/linkpath record can carry a -// NUL that Go's archive/tar does not necessarily strip.) -func rejectNULInPath(fields ...string) error { - for _, f := range fields { - if strings.IndexByte(f, 0) >= 0 { - return fmt.Errorf("Refusing entry with embedded NUL byte in path %q", f) - } - } - return nil -} - -// ensureRealParentWithinBase defends against writing through a symlink that -// already exists in the destination tree (for example, extracting into a -// directory that legitimately contains a symlink). It resolves the deepest -// already-existing ancestor directory of destPath, following every symlink, -// and refuses when that real location falls outside the destination root. -func ensureRealParentWithinBase(destPath, base string) error { - realBase, err := filepath.EvalSymlinks(base) - if err != nil { - return fmt.Errorf("Error resolving destination %s: %w", base, err) - } - realBaseWithSep := ensureTrailingSeparator(realBase) - - ancestor := filepath.Dir(destPath) - for { - real, err := filepath.EvalSymlinks(ancestor) - if err == nil { - return ensureWithinBase(real, realBase, realBaseWithSep) - } - if !errors.Is(err, os.ErrNotExist) { - return err - } - parent := filepath.Dir(ancestor) - if parent == ancestor { - return nil - } - ancestor = parent - } -} - type zipExtractOptions struct { zipWriteOptions stripComponents int @@ -5339,6 +5303,11 @@ func extractZipArchive(zipPath, destDir string, options zipExtractOptions) error if err := os.MkdirAll(absDest, 0755); err != nil { return fmt.Errorf("Error creating destination %s: %w", absDest, err) } + root, err := os.OpenRoot(absDest) + if err != nil { + return fmt.Errorf("Error opening destination %s: %w", absDest, err) + } + defer root.Close() baseWithSep := ensureTrailingSeparator(absDest) budget := newByteBudget(options.maxBytes) @@ -5370,13 +5339,13 @@ func extractZipArchive(zipPath, destDir string, options zipExtractOptions) error continue } - target := filepath.Join(absDest, filepath.FromSlash(stripped)) - target = filepath.Clean(target) + // Lexical containment as a cheap first layer; os.Root is authoritative. + target := filepath.Clean(filepath.Join(absDest, filepath.FromSlash(stripped))) if err := ensureWithinBase(target, absDest, baseWithSep); err != nil { return err } - if err := writeZipFileToDisk(file, target, options.zipWriteOptions, true, absDest, budget); err != nil { + if err := writeZipEntryToRoot(root, absDest, file, stripped, options.zipWriteOptions, budget); err != nil { return err } } @@ -5457,6 +5426,11 @@ func extractZipDirectoryEntries(files []*zip.File, targetName, destPath string, } } + root, err := os.OpenRoot(absDest) + if err != nil { + return fmt.Errorf("Error opening destination %s: %w", absDest, err) + } + defer root.Close() baseWithSep := ensureTrailingSeparator(absDest) prefix := targetName + "/" budget := newByteBudget(options.maxBytes) @@ -5479,13 +5453,12 @@ func extractZipDirectoryEntries(files []*zip.File, targetName, destPath string, continue } - target := filepath.Join(absDest, filepath.FromSlash(relative)) - target = filepath.Clean(target) + target := filepath.Clean(filepath.Join(absDest, filepath.FromSlash(relative))) if err := ensureWithinBase(target, absDest, baseWithSep); err != nil { return err } - if err := writeZipFileToDisk(file, target, options.zipWriteOptions, true, absDest, budget); err != nil { + if err := writeZipEntryToRoot(root, absDest, file, relative, options.zipWriteOptions, budget); err != nil { return err } } @@ -5514,48 +5487,44 @@ func extractZipFileEntry(file *zip.File, destPath string, options zipExtractEntr } } - return writeZipFileToDisk(file, absDest, options.zipWriteOptions, false, parent, newByteBudget(options.maxBytes)) + root, err := os.OpenRoot(parent) + if err != nil { + return fmt.Errorf("Error opening destination %s: %w", parent, err) + } + defer root.Close() + + return writeZipEntryToRoot(root, parent, file, filepath.Base(absDest), options.zipWriteOptions, newByteBudget(options.maxBytes)) } -func writeZipFileToDisk(file *zip.File, destPath string, options zipWriteOptions, ensureParents bool, base string, budget *byteBudget) error { +// writeZipEntryToRoot writes a zip entry (directory or regular file — zip +// symlinks are refused by the callers) at the root-relative path relSlash. All +// filesystem operations go through root, so the write cannot escape the +// destination. base is used only for error-message paths. +func writeZipEntryToRoot(root *os.Root, base string, file *zip.File, relSlash string, options zipWriteOptions, budget *byteBudget) error { if err := rejectNULInPath(file.Name); err != nil { return err } - if err := ensureRealParentWithinBase(destPath, base); err != nil { - return err - } - + rel := filepath.FromSlash(relSlash) + display := filepath.Join(base, rel) info := file.FileInfo() if info.IsDir() { - if ensureParents { - if err := os.MkdirAll(destPath, 0755); err != nil { - return fmt.Errorf("Error creating directory %s: %w", destPath, err) - } - } else if err := os.Mkdir(destPath, 0755); err != nil && !errors.Is(err, os.ErrExist) { - return fmt.Errorf("Error creating directory %s: %w", destPath, err) + if err := root.MkdirAll(rel, 0755); err != nil { + return fmt.Errorf("Error creating directory %s: %w", display, err) } - if options.preservePermissions { - if err := os.Chmod(destPath, info.Mode()); err != nil && !errors.Is(err, os.ErrPermission) { - return fmt.Errorf("Error setting permissions on %s: %w", destPath, err) + if err := root.Chmod(rel, info.Mode().Perm()); err != nil && !errors.Is(err, os.ErrPermission) { + return fmt.Errorf("Error setting permissions on %s: %w", display, err) } } return nil } - parentDir := filepath.Dir(destPath) - if ensureParents { - if err := os.MkdirAll(parentDir, 0755); err != nil { - return fmt.Errorf("Error creating parent directory %s: %w", parentDir, err) - } - } else { - if _, err := os.Stat(parentDir); err != nil { - return fmt.Errorf("Parent directory %s does not exist", parentDir) - } + if err := mkdirAllParent(root, relSlash, display); err != nil { + return err } - outFile, err := createExtractedFile(destPath, info.Mode().Perm(), options) + outFile, err := createExtractedFileInRoot(root, rel, info.Mode().Perm(), options, display) if err != nil { return err } @@ -5575,10 +5544,9 @@ func writeZipFileToDisk(file *zip.File, destPath string, options zipWriteOptions } if options.preservePermissions { - // Chmod via the open descriptor, not the path, so permissions can - // never be redirected onto a symlink target. + // Chmod via the open descriptor, not the path. if err := outFile.Chmod(info.Mode()); err != nil && !errors.Is(err, os.ErrPermission) { - return fmt.Errorf("Error setting permissions on %s: %w", destPath, err) + return fmt.Errorf("Error setting permissions on %s: %w", display, err) } } return nil diff --git a/mshell/Tar.go b/mshell/Tar.go index 70ecc7f..d3a9567 100644 --- a/mshell/Tar.go +++ b/mshell/Tar.go @@ -363,9 +363,11 @@ func collectTarMetadata(tarPath string) ([]tarEntryMetadata, error) { return entries, nil } -// extractTarArchive extracts an entire tarball, honoring the same option set as -// extractZipArchive (overwrite/skipExisting/stripComponents/pattern/ -// preservePermissions). Symlinks are recreated with an escape guard. +// extractTarArchive extracts an entire tarball into destDir. Every write goes +// through an os.Root anchored at destDir, so the kernel refuses any path that +// escapes the destination (via ".." or a symlink component), race-free. +// Options overwrite/skipExisting/stripComponents/pattern/preservePermissions/ +// maxBytes are honored; symlinks are recreated with an escape guard. func extractTarArchive(tarPath, destDir string, options zipExtractOptions) error { reader, closer, err := openTarReader(tarPath) if err != nil { @@ -380,6 +382,12 @@ func extractTarArchive(tarPath, destDir string, options zipExtractOptions) error if err := os.MkdirAll(absDest, 0755); err != nil { return fmt.Errorf("Error creating destination %s: %w", absDest, err) } + root, err := os.OpenRoot(absDest) + if err != nil { + return fmt.Errorf("Error opening destination %s: %w", absDest, err) + } + defer root.Close() + baseWithSep := ensureTrailingSeparator(absDest) budget := newByteBudget(options.maxBytes) @@ -415,13 +423,14 @@ func extractTarArchive(tarPath, destDir string, options zipExtractOptions) error continue } - target := filepath.Join(absDest, filepath.FromSlash(stripped)) - target = filepath.Clean(target) + // Lexical containment as a cheap first layer; os.Root is the + // authoritative, kernel-enforced check performed by the write below. + target := filepath.Clean(filepath.Join(absDest, filepath.FromSlash(stripped))) if err := ensureWithinBase(target, absDest, baseWithSep); err != nil { return err } - if err := writeTarEntryToDisk(reader, header, target, options.zipWriteOptions, true, absDest, baseWithSep, budget); err != nil { + if err := writeTarEntryToRoot(root, absDest, reader, header, stripped, options.zipWriteOptions, budget); err != nil { return err } } @@ -431,7 +440,8 @@ func extractTarArchive(tarPath, destDir string, options zipExtractOptions) error // extractTarEntry extracts a single named entry (a file or a directory subtree) // mirroring extractZipEntry. Because tar is a stream format it makes a single -// pass, collecting the file entry or the subtree as it goes. +// pass, collecting the file entry or the subtree as it goes. Writes are +// performed through an os.Root anchored at the destination. func extractTarEntry(tarPath, entryPath, destPath string, options zipExtractEntryOptions) error { targetName := normalizeZipEntryName(entryPath) if targetName == "" { @@ -452,7 +462,12 @@ func extractTarEntry(tarPath, entryPath, destPath string, options zipExtractEntr } fileFound := false - dirCreated := false + var root *os.Root + defer func() { + if root != nil { + root.Close() + } + }() baseWithSep := "" budget := newByteBudget(options.maxBytes) @@ -483,7 +498,7 @@ func extractTarEntry(tarPath, entryPath, destPath string, options zipExtractEntr } // Directory subtree: dest is a directory that receives the subtree. - if !dirCreated { + if root == nil { if options.mkdirs { if err := os.MkdirAll(absDest, 0755); err != nil { return fmt.Errorf("Error creating destination %s: %w", absDest, err) @@ -497,8 +512,12 @@ func extractTarEntry(tarPath, entryPath, destPath string, options zipExtractEntr return fmt.Errorf("Destination %s is not a directory", absDest) } } + r, err := os.OpenRoot(absDest) + if err != nil { + return fmt.Errorf("Error opening destination %s: %w", absDest, err) + } + root = r baseWithSep = ensureTrailingSeparator(absDest) - dirCreated = true } if isSelfDir { @@ -510,18 +529,17 @@ func extractTarEntry(tarPath, entryPath, destPath string, options zipExtractEntr continue } - target := filepath.Join(absDest, filepath.FromSlash(relative)) - target = filepath.Clean(target) + target := filepath.Clean(filepath.Join(absDest, filepath.FromSlash(relative))) if err := ensureWithinBase(target, absDest, baseWithSep); err != nil { return err } - if err := writeTarEntryToDisk(reader, header, target, options.zipWriteOptions, true, absDest, baseWithSep, budget); err != nil { + if err := writeTarEntryToRoot(root, absDest, reader, header, relative, options.zipWriteOptions, budget); err != nil { return err } } - if !fileFound && !dirCreated { + if !fileFound && root == nil { return fmt.Errorf("Entry '%s' not found in %s", entryPath, tarPath) } return nil @@ -539,104 +557,104 @@ func extractTarSingleFile(reader *tar.Reader, header *tar.Header, absDest string } } - baseWithSep := ensureTrailingSeparator(parent) - return writeTarEntryToDisk(reader, header, absDest, options.zipWriteOptions, false, parent, baseWithSep, budget) + root, err := os.OpenRoot(parent) + if err != nil { + return fmt.Errorf("Error opening destination %s: %w", parent, err) + } + defer root.Close() + + return writeTarEntryToRoot(root, parent, reader, header, filepath.Base(absDest), options.zipWriteOptions, budget) } -// writeTarEntryToDisk writes one tar entry (directory, file, or symlink) to -// destPath. For symlinks it validates that the link target cannot escape the -// destination root before creating it. The archive-symlink target guard and -// the ensureRealParentWithinBase guard together stop a path component from -// redirecting a write outside the destination. -func writeTarEntryToDisk(reader *tar.Reader, header *tar.Header, destPath string, options zipWriteOptions, ensureParents bool, base, baseWithSep string, budget *byteBudget) error { +// writeTarEntryToRoot writes one tar entry (directory, file, or symlink) at the +// root-relative path relSlash (forward-slash form). All filesystem operations +// go through root, so the kernel guarantees the write cannot escape the +// destination via ".." or a symlink component. base is used only to build +// human-readable paths for error messages. +func writeTarEntryToRoot(root *os.Root, base string, reader *tar.Reader, header *tar.Header, relSlash string, options zipWriteOptions, budget *byteBudget) error { if err := rejectNULInPath(header.Name, header.Linkname); err != nil { return err } - if err := ensureRealParentWithinBase(destPath, base); err != nil { - return err - } + rel := filepath.FromSlash(relSlash) + display := filepath.Join(base, rel) switch header.Typeflag { case tar.TypeDir: - if ensureParents { - if err := os.MkdirAll(destPath, 0755); err != nil { - return fmt.Errorf("Error creating directory %s: %w", destPath, err) - } - } else if err := os.Mkdir(destPath, 0755); err != nil && !errors.Is(err, os.ErrExist) { - return fmt.Errorf("Error creating directory %s: %w", destPath, err) + if err := root.MkdirAll(rel, 0755); err != nil { + return fmt.Errorf("Error creating directory %s: %w", display, err) } if options.preservePermissions { - if err := os.Chmod(destPath, header.FileInfo().Mode().Perm()); err != nil && !errors.Is(err, os.ErrPermission) { - return fmt.Errorf("Error setting permissions on %s: %w", destPath, err) + if err := root.Chmod(rel, header.FileInfo().Mode().Perm()); err != nil && !errors.Is(err, os.ErrPermission) { + return fmt.Errorf("Error setting permissions on %s: %w", display, err) } } return nil case tar.TypeSymlink: - return writeTarSymlink(header, destPath, options, ensureParents, base, baseWithSep) + return writeTarSymlinkToRoot(root, base, header, relSlash, options) case tar.TypeReg, '\x00': // '\x00' is the legacy TypeRegA (deprecated) regular-file flag - return writeTarRegularFile(reader, header, destPath, options, ensureParents, budget) + return writeTarRegularFileToRoot(root, base, reader, header, relSlash, options, budget) default: return fmt.Errorf("tarExtract cannot handle entry %s (unsupported type %q)", header.Name, string(header.Typeflag)) } } -func writeTarSymlink(header *tar.Header, destPath string, options zipWriteOptions, ensureParents bool, base, baseWithSep string) error { - // Resolve the link target relative to the symlink's own directory and - // ensure it stays within the destination root. - linkDir := filepath.Dir(destPath) - var resolved string - if filepath.IsAbs(header.Linkname) { - resolved = filepath.Clean(header.Linkname) - } else { - resolved = filepath.Clean(filepath.Join(linkDir, filepath.FromSlash(header.Linkname))) +// mkdirAllParent creates the parent directories of a root-relative slash path. +func mkdirAllParent(root *os.Root, relSlash, display string) error { + dir := path.Dir(relSlash) + if dir == "." || dir == "/" { + return nil } - if err := ensureWithinBase(resolved, base, baseWithSep); err != nil { + if err := root.MkdirAll(filepath.FromSlash(dir), 0755); err != nil { + return fmt.Errorf("Error creating parent directory for %s: %w", display, err) + } + return nil +} + +func writeTarSymlinkToRoot(root *os.Root, base string, header *tar.Header, relSlash string, options zipWriteOptions) error { + rel := filepath.FromSlash(relSlash) + display := filepath.Join(base, rel) + + // os.Root blocks *traversal* through an escaping symlink at use time but + // does not validate a link target at creation, so refuse escaping targets + // here to fail closed and match the zip behavior. + if symlinkTargetEscapes(header.Linkname, relSlash) { return fmt.Errorf("Refusing to extract symlink %s pointing outside destination (%s)", header.Name, header.Linkname) } - parentDir := filepath.Dir(destPath) - if ensureParents { - if err := os.MkdirAll(parentDir, 0755); err != nil { - return fmt.Errorf("Error creating parent directory %s: %w", parentDir, err) - } - } else if _, err := os.Stat(parentDir); err != nil { - return fmt.Errorf("Parent directory %s does not exist", parentDir) + if err := mkdirAllParent(root, relSlash, display); err != nil { + return err } - if _, err := os.Lstat(destPath); err == nil { + if _, err := root.Lstat(rel); err == nil { if options.skipExisting { return nil } if !options.overwrite { - return fmt.Errorf("Destination %s already exists", destPath) + return fmt.Errorf("Destination %s already exists", display) } - if err := os.Remove(destPath); err != nil { - return fmt.Errorf("Error replacing %s: %w", destPath, err) + if err := root.Remove(rel); err != nil { + return fmt.Errorf("Error replacing %s: %w", display, err) } } else if !errors.Is(err, os.ErrNotExist) { return err } - if err := os.Symlink(header.Linkname, destPath); err != nil { - return fmt.Errorf("Error creating symlink %s: %w", destPath, err) + if err := root.Symlink(header.Linkname, rel); err != nil { + return fmt.Errorf("Error creating symlink %s: %w", display, err) } return nil } -func writeTarRegularFile(reader *tar.Reader, header *tar.Header, destPath string, options zipWriteOptions, ensureParents bool, budget *byteBudget) error { - parentDir := filepath.Dir(destPath) - if ensureParents { - if err := os.MkdirAll(parentDir, 0755); err != nil { - return fmt.Errorf("Error creating parent directory %s: %w", parentDir, err) - } - } else { - if _, err := os.Stat(parentDir); err != nil { - return fmt.Errorf("Parent directory %s does not exist", parentDir) - } +func writeTarRegularFileToRoot(root *os.Root, base string, reader *tar.Reader, header *tar.Header, relSlash string, options zipWriteOptions, budget *byteBudget) error { + rel := filepath.FromSlash(relSlash) + display := filepath.Join(base, rel) + + if err := mkdirAllParent(root, relSlash, display); err != nil { + return err } mode := header.FileInfo().Mode().Perm() - outFile, err := createExtractedFile(destPath, mode, options) + outFile, err := createExtractedFileInRoot(root, rel, mode, options, display) if err != nil { return err } @@ -650,15 +668,27 @@ func writeTarRegularFile(reader *tar.Reader, header *tar.Header, destPath string } if options.preservePermissions { - // Chmod via the open descriptor, not the path, so permissions can - // never be redirected onto a symlink target. + // Chmod via the open descriptor, not the path. if err := outFile.Chmod(mode); err != nil && !errors.Is(err, os.ErrPermission) { - return fmt.Errorf("Error setting permissions on %s: %w", destPath, err) + return fmt.Errorf("Error setting permissions on %s: %w", display, err) } } return nil } +// symlinkTargetEscapes reports whether a symlink target would point outside the +// extraction root, given the link's own root-relative location (slash form). +// Absolute targets, and relative targets that resolve above the root, are +// refused. The target is not otherwise simplified — its meaning depends on +// where the link lives. +func symlinkTargetEscapes(linkname, relSlash string) bool { + if strings.HasPrefix(filepath.ToSlash(linkname), "/") || filepath.IsAbs(linkname) { + return true + } + joined := path.Join(path.Dir(relSlash), filepath.ToSlash(linkname)) + return joined == ".." || strings.HasPrefix(joined, "../") +} + // readTarEntry reads a single entry's bytes without writing to disk, mirroring // readZipEntry. Returns found=false when the entry is absent. func readTarEntry(tarPath, entryPath string) ([]byte, bool, error) {