Skip to content

Fix path traversal attack in -proto-out-dir - #573

Merged
dragonsinth merged 3 commits into
fullstorydev:masterfrom
bcleenders:fix-path-traversal
Jul 27, 2026
Merged

Fix path traversal attack in -proto-out-dir#573
dragonsinth merged 3 commits into
fullstorydev:masterfrom
bcleenders:fix-path-traversal

Conversation

@bcleenders

@bcleenders bcleenders commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

A malicious file descriptor name (e.g. from an untrusted protoset or server reflection) could escape the output directory through ../ sequences, writing attacker-controlled content to arbitrary paths.

Confine all writes to an os.Root opened on the output directory.


Note

Low Risk
Security-hardening change with targeted tests; behavior for legitimate relative proto paths should be unchanged while blocking traversal.

Overview
Fixes a path traversal vulnerability when exporting protos via -proto-out-dir: descriptor names from untrusted protosets or server reflection could use ../ and write outside the chosen output directory.

Proto export now opens an os.Root on the output directory and performs all MkdirAll / Create operations through it, so malicious names like ../escape.proto or nested foo/../../../escape.proto are rejected instead of escaping the sandbox. The write loop is factored into writeProtoFiles, and directory creation uses 0755 instead of 0777.

Tests cover a normal nested path (foo/bar.proto) and assert traversal attempts fail and do not create files outside the temp output dir.

Reviewed by Cursor Bugbot for commit c4fbb35. Bugbot is set up for automated code reviews on this repo. Configure here.

bcleenders and others added 3 commits July 27, 2026 11:02
A malicious file descriptor name (e.g. from an untrusted protoset or
server reflection) could escape the output directory through ../
sequences, writing attacker-controlled content to arbitrary paths.

Confine all writes to an `os.Root` opened on the output directory.

@dragonsinth dragonsinth left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after a few changes; I wanted to actually traverse the live-code root

@dragonsinth
dragonsinth merged commit 6be36ba into fullstorydev:master Jul 27, 2026
3 of 4 checks passed
@bcleenders
bcleenders deleted the fix-path-traversal branch August 4, 2026 07:56
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