diff --git a/github/github.go b/github/github.go index 289abcc49fe..28329012864 100644 --- a/github/github.go +++ b/github/github.go @@ -884,14 +884,12 @@ func (c *Client) NewFormRequest(ctx context.Context, urlStr string, body io.Read } // checkURLPathTraversal returns ErrPathForbidden if urlStr contains ".." as a -// path segment (e.g. "a/../b"), preventing path traversal attacks. It does not -// match ".." embedded within a segment (e.g. "file..txt"). The check is -// performed only on the path portion of the URL, ignoring any query string or -// fragment. +// path segment (e.g. "a/../b"), preventing path traversal attacks. Percent- +// encoded equivalents such as "%2e%2e" are also rejected because url.Parse +// decodes them to ".." before the check runs. It does not match ".." embedded +// within a segment (e.g. "file..txt"). The check is performed only on the path +// portion of the URL, ignoring any query string or fragment. func checkURLPathTraversal(urlStr string) error { - if !strings.Contains(urlStr, "..") { - return nil - } u, err := url.Parse(urlStr) if err != nil { return err diff --git a/github/github_test.go b/github/github_test.go index b16ac1ffd87..1b7417ab0ca 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -1680,6 +1680,12 @@ func TestCheckURLPathTraversal(t *testing.T) { // URL with userinfo. {"https://user:pass@api.github.com/repos/../admin", ErrPathForbidden}, {"https://user:pass@api.github.com/repos/o/r", nil}, + // Percent-encoded dots (%2e%2e) — url.Parse decodes them to ".." in Path. + {"repos/%2e%2e/admin", ErrPathForbidden}, + {"repos/%2E%2E/admin", ErrPathForbidden}, + {"repos/x/%2e%2e/%2e%2e/%2e%2e/admin", ErrPathForbidden}, + {"x/%2e%2e/%2e%2e/%2e%2e/admin/users", ErrPathForbidden}, + {"repos/o/r/contents/file%2e%2etxt", nil}, } for _, tt := range tests { err := checkURLPathTraversal(tt.input)