fix: path-traversal in extraction, honest GPLv2 disclosure, CI hardening - #1
Merged
Merged
Conversation
Archive entry names are attacker-controlled. CreateNewFile combined the entry name into the output directory without containment, so an entry named with ..\ segments or an absolute path could write outside the chosen output directory (arbitrary file write). Resolve the output root and the target path to full paths and reject any target that does not sit under the output root (case-insensitive, with a trailing separator to avoid sibling-prefix bypass). The thrown UnauthorizedAccessException is caught by the per-entry catch (Exception) in Archive_ExtractFiles, so a malicious entry is skipped and the process exits non-zero rather than crashing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…k -if
(a) In the non-archive path, the "Converting <fmt> audio" progress line
and the "All OK" success line were written to stdout, contaminating
the clean-stdout contract that parent processes rely on. Route both
to stderr (the archive path already does this).
(b) The -if switch read args[i] without checking for a missing value,
throwing IndexOutOfRangeException. Add the same bounds check already
used by -o and -f: PrintError + return when the value is absent.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… emit sha256, fix cleanup glob - Add permissions: contents: write so the release upload does not 403. - Scope the tag trigger to v0.* so the ~50 inherited upstream v1.x tags no longer fire this fork's release workflow. - Fix the debug-symbol cleanup: Get-ChildItem -Include only matches when the path ends in \*, so use pkg\* instead of pkg (previously a no-op). - Bundle LICENSE, NOTICE and licenses/GPL-2.0.txt into the zip (the shipped ArcFormats.dll contains GPLv2 sources, so the GPL text must ship with the binary). Cleanup runs before this so it can't sweep them. - Compute the archive SHA-256 and write it to the step summary and the release body. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…2.0 text The prebuilt ArcFormats.dll links in two GPLv2 source files inherited from upstream (Blowfish.cs, KogadoCocotte.cs), so the release archive as a whole is a combined work under GPLv2 — not "MIT-clean" as previously claimed. - README: replace the "every bundled dependency is permissively licensed / MIT-clean" claim (both English and 繁中) with an honest statement — CLI packaging and fork code are MIT, but the release binary is GPLv2 because of the two upstream files; arm's-length subprocess calls are unaffected. - README/NOTICE: drop Concentus and System.Data.SQLite from the CLI tech stack / credits (GUI/Experimental only, not in the CLI release zip). - NOTICE: add a "Bundled GPLv2 components" disclosure section. - Add licenses/GPL-2.0.txt (verbatim FSF GPL v2 text). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes from a security & license audit of the CLI and its release pipeline.
🔴 Security (Critical) — path traversal / arbitrary file write
CreateNewFilecombined the archive entry name with the output directory with no containment check, so a crafted archive whose entry names contain..\..\or an absolute path could write outside-o(e.g. drop a file into Startup, overwrite arbitrary files). The threat model for an extractor is untrusted archive content, so this is exploitable with a single malicious pack.Added a containment guard: resolve the full output root + target path and reject anything not under the root (with a trailing-separator check to block
C:\out→C:\out-evilsibling-prefix bypass). Malicious entries are now skipped and the process exits non-zero instead of writing out of bounds.Console/ConsoleBrowser.cs.CLI correctness
Converting…,All OK) to stdout; moved to stderr soxkeeps stdout clean for scripted use (the archive path already did this).-if <format>with a missing value could throwIndexOutOfRangeException; added a bounds check matching the existing-o/-fhandling.License hygiene — honest GPLv2 disclosure
The bundled
ArcFormats.dllcompiles two GPLv2 files inherited from upstream GARbro —ArcFormats/Blowfish.cs(© 2007 Novell) andArcFormats/KogadoCocotte.cs(C# port by morkt of GPLv2 code by juicy.gt) — so the combined binary is a GPLv2 work, not "MIT-clean". Replaced the prior "MIT-clean / every bundled dependency is permissively licensed" wording (EN + 繁中) inREADME.mdandNOTICEwith an accurate statement, addedlicenses/GPL-2.0.txt, and clarified that arm's-length invocation of the executable does not place callers under the GPL. All code added by this fork, and the CLI packaging, remain MIT.Also corrected dependencies listed as bundled but not actually in the CLI zip (Concentus, System.Data.SQLite — GUI/Experimental only).
CI / release (
.github/workflows/release.yml)permissions: contents: write(release upload would 403 otherwise).v*→v0.*so the ~50 inherited upstreamv1.xtags can't mass-trigger broken releases ongit push --tags.LICENSE,NOTICE,licenses/GPL-2.0.txtinto the release zip; fixed the ineffective.pdb/.xmlcleanup glob; emit the zip SHA-256 to the job summary and release body.Testing
x -y -q -f "SCRIPT\.SRC|TEXT\.DAT|POINT\.DAT"on a real SoftPaldata.pac→ the three extracted files are SHA-256-identical to the known-good baseline (extraction unchanged by the security fix)...\..\, absolute, and sibling-prefix paths blocked.The security + CLI-correctness commits are also submitted upstream (continues morkt#670).
🤖 Generated with Claude Code