File tree Expand file tree Collapse file tree
Expand file tree Collapse file tree Original file line number Diff line number Diff line change @@ -1054,11 +1054,13 @@ async def get_resolved_ref(self):
10541054 if hasattr (self , "resolved_ref" ):
10551055 return self .resolved_ref
10561056
1057+ # Encode the ref (safe="" so its slashes can't inject extra path
1058+ # segments) to prevent traversal, see GHSA-q276-fxp7-xhx3.
10571059 api_url = "{api_base_path}/repos/{user}/{repo}/commits/{ref}" .format (
10581060 api_base_path = self .api_base_path .format (hostname = self .hostname ),
10591061 user = self .user ,
10601062 repo = self .repo ,
1061- ref = self .unresolved_ref ,
1063+ ref = urllib . parse . quote ( self .unresolved_ref , safe = "" ) ,
10621064 )
10631065 self .log .debug ("Fetching %s" , api_url )
10641066 cached = self .cache .get (api_url )
Original file line number Diff line number Diff line change 11import re
22from unittest import TestCase
3- from urllib .parse import quote
3+ from urllib .parse import quote , urlparse
44
55import pytest
66from tornado .ioloop import IOLoop
@@ -417,6 +417,26 @@ def test_github_missing_ref():
417417 assert ref is None
418418
419419
420+ async def test_github_ref_is_url_encoded (monkeypatch ):
421+ """An attacker-controlled ref must not inject extra GitHub API path segments."""
422+ ref = "HEAD/../../../../owner/private/contents/README.md"
423+ provider = GitHubRepoProvider (spec = f"user/repo/{ ref } " )
424+
425+ captured = {}
426+
427+ async def fake_request (api_url , etag = None ):
428+ captured ["url" ] = api_url
429+ return None # short-circuit before any network call
430+
431+ monkeypatch .setattr (provider , "github_api_request" , fake_request )
432+ await provider .get_resolved_ref ()
433+
434+ path = urlparse (captured ["url" ]).path
435+ # the ref's slashes are encoded, so the ".." cannot climb out of /commits/
436+ assert path == f"/repos/user/repo/commits/{ quote (ref , safe = '' )} "
437+ assert "/../" not in path
438+
439+
420440class TestSpecErrorHandling (TestCase ):
421441 def test_too_short_spec (self ):
422442 spec = "nothing_to_split"
You can’t perform that action at this time.
0 commit comments