Skip to content

fix(resourceinfo): reject manifest paths escaping the destination#23

Open
Rakdos8 wants to merge 1 commit into
carbonengine:mainfrom
Rakdos8:fix/zip-slip-path-containment
Open

fix(resourceinfo): reject manifest paths escaping the destination#23
Rakdos8 wants to merge 1 commit into
carbonengine:mainfrom
Rakdos8:fix/zip-slip-path-containment

Conversation

@Rakdos8

@Rakdos8 Rakdos8 commented May 15, 2026

Copy link
Copy Markdown

Summary

Closes a zip-slip / path-traversal arbitrary file write in the resource
unpack path.

Problem

m_relativePath comes straight from the manifest and was joined onto
basePath and written (5 Put* sites) with no containment check. A
manifest entry such as ../../x or an absolute path wrote files anywhere
on disk.

Fix

Add a file-local IsPathWithinBase helper (weakly_canonical +
lexically_relative, with a lexical-normalise fallback) and guard every
write site, returning the existing MALFORMED_RESOURCE_INPUT result when
the target escapes basePath.

Type

Security — path traversal / arbitrary file write (Critical).

Testing

Manual review; legitimate relative paths resolve within base and are
unaffected; ../absolute escapes are now refused.

@CCP-Aporia

Copy link
Copy Markdown

Hi @Rakdos8

Thank you for the PR and apologies for the late response. We are still in the process of sorting out some details around contribution guidelines and PR templates.

It would be great if you could add a test case for this problem to the test suite.

m_relativePath comes straight from the manifest and was joined onto
basePath and written with no containment check, so an entry like
'../../x' (zip-slip) wrote files anywhere on disk. Add a containment
check (weakly_canonical + lexically_relative, lexical fallback) and
refuse any Put* whose target escapes basePath.
@Rakdos8 Rakdos8 force-pushed the fix/zip-slip-path-containment branch from 39d576e to ee3ded3 Compare June 10, 2026 22:41
@Rakdos8

Rakdos8 commented Jun 10, 2026

Copy link
Copy Markdown
Author

Hi @Rakdos8

Thank you for the PR and apologies for the late response. We are still in the process of sorting out some details around contribution guidelines and PR templates.

It would be great if you could add a test case for this problem to the test suite.

No worries about the delay, and thanks for taking a look!

I've added a test case for this: UnpackBundleRejectsZipSlipRelativePath in tests/src/ResourcesLibraryTest.cpp, along
with a small manifest fixture under tests/testData/ZipSlip/.

It exercises the real-world vector end-to-end through the public API:

  1. Imports a ResourceGroup whose single resource has a RelativePath that escapes the destination (../zipSlipEscaped.txt).
  2. Bundles it (reading the data from the local CDN by location, so the malicious path only takes effect at unpack
    time).
  3. Unpacks the bundle and asserts that Unpack returns MALFORMED_RESOURCE_INPUT and that no file is written outside the destination base path.

Without the containment check the unpack would write zipSlipEscaped.txt outside the destination; with it, the put is refused before any data is written.

One note: I couldn't run the suite locally (the CMake presets target Windows/macOS and I'm on Linux). Happy to adjust the test placement or style to match your conventions once the contribution guidelines are finalized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants