Skip to content

Prevent URI traversal for GitHub refs - #2122

Open
Yann-P wants to merge 1 commit into
jupyterhub:mainfrom
Yann-P:fix-traversal
Open

Prevent URI traversal for GitHub refs#2122
Yann-P wants to merge 1 commit into
jupyterhub:mainfrom
Yann-P:fix-traversal

Conversation

@Yann-P

@Yann-P Yann-P commented Jul 22, 2026

Copy link
Copy Markdown

This defect exposes an oracle for existence of private repos or refs through the Binder operator's github token.

Fixes
https://github.com/jupyterhub/binderhub/security/advisories/GHSA-q276-fxp7-xhx3.

I am sending this fix through a regular PR and not a private GHSA fork, because the vulnerability is not severe, and private forks have a lot of friction (no CI, 500 errors, easy to miss when releasing).

Checklist

  • I am the author of this work

What does this PR do?

Type of change:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation
  • Other

Is this PR related to an issue, or is it part of a larger body of work?

https://github.com/jupyterhub/binderhub/security/advisories/GHSA-q276-fxp7-xhx3

Does this PR introduce a breaking change?

No

How can this PR be tested?

Claude-generated end to end test procedure

File 1 — listener.py (fake api.github.com)

from http.server import BaseHTTPRequestHandler, HTTPServer

class H(BaseHTTPRequestHandler):
    def do_GET(self):
        print(">>> WIRE PATH :", self.path, flush=True)
        print(">>> AUTH HDR  :", self.headers.get("Authorization"), flush=True)
        self.send_response(404); self.end_headers()
        self.wfile.write(b'{"message":"Not Found"}')
    def log_message(self, *a): pass

HTTPServer(("127.0.0.1", 8000), H).serve_forever()

File 2 — bhub_config.py (standalone BinderHub, GitHub provider aimed at the listener)

from binderhub.build import FakeBuild
from binderhub.registry import FakeRegistry
from binderhub.repoproviders import GitHubRepoProvider

c.BinderHub.builder_required = False
c.BinderHub.use_registry = True
c.BinderHub.registry_class = FakeRegistry
c.BinderHub.build_class = FakeBuild
c.BinderHub.repo_providers = {"gh": GitHubRepoProvider}

c.GitHubRepoProvider.hostname = "127.0.0.1:8000"
c.GitHubRepoProvider.api_base_path = "http://{hostname}"
c.GitHubRepoProvider.access_token = "SECRET-OPERATOR-TOKEN"

(Auth is off by default → the route is anonymous, exactly the default deployment mode.)

Run

python listener.py &                              # terminal 1
python -m binderhub -f bhub_config.py &            # terminal 2  (serves on :8585)
# wait until up:
until curl -s -o /dev/null http://127.0.0.1:8585/health; do sleep 1; done

Fire the two attacks

--path-as-is is required so curl doesn't collapse the .. itself — we want BinderHub's tornado to receive the raw path. The Accept: text/event-stream header is required or the build handler 400s.

# 1) ref injection
curl -s --path-as-is -H "Accept: text/event-stream" \
  "http://127.0.0.1:8585/build/gh/a/b/HEAD/../../../../OWNER/PRIVREPO/contents/README.md"

# 2) ".." owner pivot
curl -s --path-as-is -H "Accept: text/event-stream" \
  "http://127.0.0.1:8585/build/gh/../x/HEAD"

Expected results

Fixed (current HEAD):

  • Attack 1 → listener logs WIRE PATH : /repos/a/b/commits/HEAD%2F..%2F..%2F..%2F..%2FOWNER%2FPRIVREPO%2Fcontents%2FREADME.md — slashes %2F-encoded, .. inert, request stays on a/b.
  • Attack 2 → BinderHub replies "Invalid GitHub user: '..'", listener sees nothing (rejected at construction).

What should a reviewer concentrate their feedback on?

Other information

AI disclosure: tests generated by Claude Code (pytest and end to end)

@Yann-P Yann-P added the bug label Jul 22, 2026
@Yann-P
Yann-P requested a review from krassowski July 22, 2026 20:07
@arpitjain099

Copy link
Copy Markdown

Thanks for the quick turnaround. As the reporter, this closes the vector I reported: quoting the ref with safe="" percent-encodes the slashes, so it lands as one opaque path segment and libcurl has no /../ to collapse before the request goes out. Good catch also adding the "." / ".." guards on user and repo, which handle the /build/gh/../x/HEAD variant that quoting the ref alone wouldn't.

I checked the other interpolation sites too (get_repo_url, get_resolved_ref_url): they build github.com URLs rather than the token-bearing api path, and user/repo can't contain slashes and are now guarded, so I don't see a residual. That's from reading the diff rather than running the full suite.

Looks good to me.

@krassowski

Copy link
Copy Markdown

I re-run tests (v1.30, auth) that failed on the first run, it looks like it might be flaky, but if it fails second time we may need to investigate.

@krassowski

Copy link
Copy Markdown

Hmm, still failing, but I also see it is failing on main so maybe ok?

@krassowski krassowski left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks ok to me. It looks like @rgaiacs and @manics were most recently merging changes in this repo.

@rgaiacs

rgaiacs commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Dear @Yann-P,

Thanks for the pull request.

I did not fully understand the problem here and I appreciate a more concrete example of the issue. Feel free to comment in https://github.com/jupyterhub/binderhub/security/advisories/GHSA-q276-fxp7-xhx3.

Normally, the user will make a request like

https://mybinder.org/v2/gh/binder-examples/conda/f00a783146e9c6a2ed9726f01fc09fbfbad2f89e?urlpath=%2Fdoc%2Ftree%2Findex.ipynb

where

  • gh is the provider identifier for GitHub
  • binder-examples is the GitHub username
  • conda is the GitHub repository
  • f00a783146e9c6a2ed9726f01fc09fbfbad2f89e is the Git commit ID

You reported that a user with malicious intentions could make a request like

https://mybinder.org/v2/gh/binder-examples/../conda/f00a783146e9c6a2ed9726f01fc09fbfbad2f89e?urlpath=%2Fdoc%2Ftree%2Findex.ipynb

but to my knowledge, the request would not match a existing endpoint.

This defect exposes an oracle for existence of private repos or refs
through the Binder operator's github token.

Fixes
https://github.com/jupyterhub/binderhub/security/advisories/GHSA-q276-fxp7-xhx3.
Comment thread binderhub/repoproviders.py Outdated
self.user, self.repo, self.unresolved_ref = tokenize_spec(self.spec)
self.repo = strip_suffix(self.repo, ".git")

# Prevents traversal (e.g. "../repo/HEAD"), see GHSA-q276-fxp7-xhx3

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.

In addition to forbidding . and .. we could tighten up the allowed characters:

@Yann-P

Yann-P commented Jul 27, 2026

Copy link
Copy Markdown
Author

Hello @rgaiacs, I answered here: https://github.com/jupyterhub/binderhub/security/advisories/GHSA-q276-fxp7-xhx3#advisory-comment-258441

Consequently:

  • I kept only the fix with the ref (that I could reproduce locally), so @manics the code you reviewed regarding the user regex is gone
  • Closed the advisory as the attack prerequisites of running binderhub on a broad-access github token is not enough to justify a GHSA.

Let me know if this resolution works for you

@Yann-P Yann-P changed the title Prevent URI traversal for GitHub repos, users and refs Prevent URI traversal for GitHub refs Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants