Skip to content

fix(security): 2 improvements across 2 files#5281

Open
tomaioo wants to merge 2 commits intogoogle:masterfrom
tomaioo:fix/security/path-traversal-during-tar-extraction-can
Open

fix(security): 2 improvements across 2 files#5281
tomaioo wants to merge 2 commits intogoogle:masterfrom
tomaioo:fix/security/path-traversal-during-tar-extraction-can

Conversation

@tomaioo
Copy link
Copy Markdown

@tomaioo tomaioo commented Apr 24, 2026

Summary

fix(security): 2 improvements across 2 files

Problem

Severity: High | File: gcp/indexer/shared/shared.go:L57

The tar extraction logic joins tmpDir with hdr.Name and then cleans the result, but does not verify that the final path is still داخل tmpDir. A crafted tar entry such as ../../etc/passwd or an absolute path can escape the destination directory and overwrite arbitrary files on the host.

Solution

Before writing, validate each target path: reject absolute paths, reject .. traversal, and ensure strings.HasPrefix(resolvedPath, tmpDir+string(os.PathSeparator)). Also handle tar header types explicitly (regular files only) and reject symlinks/hardlinks.

Changes

  • gcp/indexer/shared/shared.go (modified)
  • go/cmd/exporter/writer.go (modified)

tomaioo added 2 commits April 23, 2026 23:13
- Security: Path traversal during tar extraction can write outside temporary directory
- Security: Arbitrary file write via unsanitized output path joining

Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com>
- Security: Path traversal during tar extraction can write outside temporary directory
- Security: Arbitrary file write via unsanitized output path joining

Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com>
@google-cla
Copy link
Copy Markdown

google-cla Bot commented Apr 24, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@another-rex
Copy link
Copy Markdown
Contributor

But... aren't we the ones writing the tars to the bucket in the first place?

@another-rex
Copy link
Copy Markdown
Contributor

I probably don't mind merging this just as a defense in depth measure though

@tomaioo
Copy link
Copy Markdown
Author

tomaioo commented Apr 24, 2026

Good question—today we do produce those tarballs, but this extractor is still parsing untrusted input at runtime (bucket contents can be replaced, stale artifacts can persist, or write credentials can be abused).
Path traversal in archive extraction is a well-known high-impact class, so we should enforce safe extraction regardless of expected producer.
This is a defense-in-depth hardening: low code cost, and it prevents a single compromised artifact from writing outside tmpDir.

3 similar comments
@tomaioo
Copy link
Copy Markdown
Author

tomaioo commented Apr 24, 2026

Good question—today we do produce those tarballs, but this extractor is still parsing untrusted input at runtime (bucket contents can be replaced, stale artifacts can persist, or write credentials can be abused).
Path traversal in archive extraction is a well-known high-impact class, so we should enforce safe extraction regardless of expected producer.
This is a defense-in-depth hardening: low code cost, and it prevents a single compromised artifact from writing outside tmpDir.

@tomaioo
Copy link
Copy Markdown
Author

tomaioo commented Apr 25, 2026

Good question—today we do produce those tarballs, but this extractor is still parsing untrusted input at runtime (bucket contents can be replaced, stale artifacts can persist, or write credentials can be abused).
Path traversal in archive extraction is a well-known high-impact class, so we should enforce safe extraction regardless of expected producer.
This is a defense-in-depth hardening: low code cost, and it prevents a single compromised artifact from writing outside tmpDir.

@tomaioo
Copy link
Copy Markdown
Author

tomaioo commented Apr 25, 2026

Good question—today we do produce those tarballs, but this extractor is still parsing untrusted input at runtime (bucket contents can be replaced, stale artifacts can persist, or write credentials can be abused).
Path traversal in archive extraction is a well-known high-impact class, so we should enforce safe extraction regardless of expected producer.
This is a defense-in-depth hardening: low code cost, and it prevents a single compromised artifact from writing outside tmpDir.

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