Skip to content

01 - Validate the repository path in one place #7

Description

@Zefek

Validate the repository path in one place

Labels: security, enhancement
Size: S · Priority: high · Blocks: #9

Problem

The repository a request refers to is currently derived twice, by two different
parsers
, and nothing guarantees they agree.

  1. MapGitHttpBackend maps /{**gitPath} and passes the catch-all straight through:
    PathInfo = "/" + gitPath — src/GitHttpBackend.AspNetCore/GitHttpBackendEndpointExtensions.cs:40-45.
    No validation of any kind happens in the library.
  2. The authorization hook decides which repository is being accessed by taking the
    first path segment — RepoFromPath in samples/GitHttpBackend.Server/Program.cs:275-284,
    used by IsAuthorized at :310-317.
  3. git-http-backend then decides which repository to actually open from the whole
    PATH_INFO, via its own enter_repo() resolution.

Per-user repository allowlists (Git:Auth:Users:*:Repos) are the mechanism that keeps
one caller from reading another caller's repository. That mechanism rests entirely on
step 2 and step 3 producing the same answer. If a path can be crafted where they differ,
authorization approves repository A while git serves repository B.

Today this most likely holds, because Kestrel decodes and removes dot segments before
routing. That is the point: the guarantee lives in an external component's normalisation
behaviour, not in this codebase, and it is not covered by a single test.

Proposed change

Add one path parser to the host-agnostic core and make every caller use it — so the
library and any sample physically cannot disagree.

In src/GitHttpBackend:

public static class GitRepositoryPath
{
    /// <summary>
    /// Splits a CGI PATH_INFO into the repository name and the remainder, rejecting
    /// anything that is not a plain repository name followed by a git service path.
    /// </summary>
    public static bool TryParse(string pathInfo, out string repository, out string rest);
}

Rules: the first segment must be a plain name (letters, digits, ., -, _), must not
be . or .., must not contain a path separator, a colon, or a null byte. No segment
anywhere may be ... Reject backslashes outright — on Windows they are path separators,
so repo\..\other matters as much as the forward-slash form.

Then:

  • MapGitHttpBackend calls it before Authorize and returns 400 Bad Request on
    failure, logging the rejected path through the existing ForLog sanitiser.
  • The sample's RepoFromPath is deleted and replaced with a call to it, so the
    authorization decision and the library's view of the path come from the same code.

NormalizeRepo (stripping a trailing .git) stays in the sample — that is a
configuration-matching convenience, not a safety property.

Acceptance criteria

  • GitRepositoryPath.TryParse exists in the core library and is unit tested.
  • MapGitHttpBackend rejects an invalid path with 400 before the Authorize hook
    and before starting the backend process.
  • The sample derives the repository name for authorization from the same function.
  • Tests cover, at minimum, rejection of: .. segments; percent-encoded ..
    (%2e%2e); percent-encoded separators (%2f, %5c); backslash separators; an
    absolute path; a Windows drive-qualified path (C:\…); a UNC path (\\host\share);
    a leading or embedded null byte; a name that is only dots.
  • Tests confirm the normal shapes still pass: /projekt.git/info/refs,
    /projekt/info/refs, /projekt.git/git-upload-pack,
    /projekt.git/git-receive-pack, and a non-ASCII repository name.
  • A valid request still reaches git unchanged — no behaviour change for the
    working case.

Notes

Non-ASCII repository names currently work and the logging code comments call that out
deliberately (GitHttpBackendEndpointExtensions.cs:100-108). The whitelist must not
quietly break them; scope the character rules to separators and traversal, not to ASCII.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions