Skip to content

Commit 65fe088

Browse files
fix(raw): reject dot-dot segments revealed by decoding encoded separators
rejectPathTraversal split components on literal "/" before checking each segment, so a segment containing an encoded separator (e.g. "%2e%2e%2fsecret.txt") decoded to "../secret.txt" instead of "..", and the check never caught it. Percent-decoding a segment can therefore introduce new "/"-separated subsegments that were invisible to the original literal split. Recursively re-split and re-check the decoded form whenever decoding changes a segment, so a ".." revealed by one or more layers of percent-decoding (including through an encoded separator, or double-encoding) is rejected regardless of where it appears. Add regression tests for encoded-separator traversal, encoded separators in other components, and double percent-encoded dot-dot segments, plus a benign percent-encoded filename case to confirm non-traversal decodes still pass through. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent a3da715 commit 65fe088

2 files changed

Lines changed: 65 additions & 11 deletions

File tree

pkg/raw/raw.go

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,23 +25,41 @@ var errPathTraversal = errors.New(`raw: path segment ".." is not allowed`)
2525
func rejectPathTraversal(components ...string) error {
2626
for _, component := range components {
2727
for segment := range strings.SplitSeq(component, "/") {
28-
if segment == "" {
29-
continue
30-
}
31-
if segment == ".." {
32-
return errPathTraversal
33-
}
34-
// Guard against percent-encoded traversal (e.g. "%2e%2e") in case
35-
// the segment is later decoded before being treated as a path
36-
// component.
37-
if decoded, err := url.PathUnescape(segment); err == nil && decoded == ".." {
38-
return errPathTraversal
28+
if err := rejectSegment(segment); err != nil {
29+
return err
3930
}
4031
}
4132
}
4233
return nil
4334
}
4435

36+
// rejectSegment reports an error if segment is, or decodes to, "..". A
37+
// percent-encoded separator (e.g. "%2f") can appear inside a single
38+
// "/"-separated segment and only becomes a "/" once decoded, revealing new
39+
// subsegments (e.g. "%2e%2e%2fsecret.txt" decodes to "../secret.txt"). To
40+
// catch that, whenever decoding changes the segment, the decoded form is
41+
// split on "/" again and each subsegment is checked recursively, so
42+
// traversal segments introduced by one or more layers of percent-decoding
43+
// are rejected regardless of where the encoded separator falls.
44+
func rejectSegment(segment string) error {
45+
if segment == "" {
46+
return nil
47+
}
48+
if segment == ".." {
49+
return errPathTraversal
50+
}
51+
decoded, err := url.PathUnescape(segment)
52+
if err != nil || decoded == segment {
53+
return nil
54+
}
55+
for subsegment := range strings.SplitSeq(decoded, "/") {
56+
if err := rejectSegment(subsegment); err != nil {
57+
return err
58+
}
59+
}
60+
return nil
61+
}
62+
4563
// GetRawClientFn is a function type that returns a RawClient instance.
4664
type GetRawClientFn func(context.Context) (*Client, error)
4765

pkg/raw/raw_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,42 @@ func TestUrlFromOpts(t *testing.T) {
254254
owner: "octocat", repo: "hello", path: "README.md",
255255
wantErr: true,
256256
},
257+
{
258+
name: "encoded separator revealing dot-dot rejected",
259+
opts: nil,
260+
owner: "octocat", repo: "hello", path: "%2e%2e%2fsecret.txt",
261+
wantErr: true,
262+
},
263+
{
264+
name: "encoded separator revealing dot-dot mid-path rejected",
265+
opts: nil,
266+
owner: "octocat", repo: "hello", path: "docs%2f..%2f..%2fsecret.txt",
267+
wantErr: true,
268+
},
269+
{
270+
name: "encoded separator in owner rejected",
271+
opts: nil,
272+
owner: "octocat%2f..", repo: "hello", path: "README.md",
273+
wantErr: true,
274+
},
275+
{
276+
name: "double percent-encoded dot-dot rejected",
277+
opts: nil,
278+
owner: "octocat", repo: "hello", path: "%252e%252e/secret.txt",
279+
wantErr: true,
280+
},
281+
{
282+
name: "double percent-encoded separator revealing dot-dot rejected",
283+
opts: nil,
284+
owner: "octocat", repo: "hello", path: "%252e%252e%252fsecret.txt",
285+
wantErr: true,
286+
},
287+
{
288+
name: "benign percent-encoded filename allowed",
289+
opts: nil,
290+
owner: "octocat", repo: "hello", path: "%2ehidden",
291+
want: "https://raw.example.com/octocat/hello/HEAD/%2ehidden",
292+
},
257293
}
258294

259295
for _, tt := range tests {

0 commit comments

Comments
 (0)