From 42513398c05ca6bdf71da76cb6f9baaebe8cb924 Mon Sep 17 00:00:00 2001 From: bircni Date: Sat, 6 Jun 2026 17:44:56 +0200 Subject: [PATCH] fix(lfs): reject unknown SSH LFS sub-verbs to prevent auth bypass (#38008) An authenticated SSH user could pass a malformed sub-verb (e.g. `git-lfs-authenticate badverb`) so getAccessMode falls through to AccessModeNone (0). The permission check in routers/private/serv.go then evaluates `userMode < 0` which is always false, granting a valid LFS JWT for any private repository. The HTTP LFS handler only validates the Op claim on writes, so the token works for downloads. Validate the sub-verb in runServ before calling getAccessMode and fail fast for anything other than upload/download. --- cmd/serv.go | 23 ++++++++++++-------- cmd/serv_test.go | 56 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 9 deletions(-) create mode 100644 cmd/serv_test.go diff --git a/cmd/serv.go b/cmd/serv.go index 93009cd32a..b39076f6a7 100644 --- a/cmd/serv.go +++ b/cmd/serv.go @@ -113,23 +113,25 @@ func handleCliResponseExtra(extra private.ResponseExtra) error { return nil } -func getAccessMode(verb, lfsVerb string) perm.AccessMode { +// getAccessMode maps an SSH git/LFS verb to the access mode it requires, with +// ok=false for an unrecognised verb. Callers MUST reject the request when ok is +// false: AccessModeNone would otherwise pass the `userMode < mode` permission +// check in routers/private/serv.go and grant access. +func getAccessMode(verb, lfsVerb string) (mode perm.AccessMode, ok bool) { switch verb { case git.CmdVerbUploadPack, git.CmdVerbUploadArchive: - return perm.AccessModeRead + return perm.AccessModeRead, true case git.CmdVerbReceivePack: - return perm.AccessModeWrite + return perm.AccessModeWrite, true case git.CmdVerbLfsAuthenticate, git.CmdVerbLfsTransfer: switch lfsVerb { case git.CmdSubVerbLfsUpload: - return perm.AccessModeWrite + return perm.AccessModeWrite, true case git.CmdSubVerbLfsDownload: - return perm.AccessModeRead + return perm.AccessModeRead, true } } - // should be unreachable - setting.PanicInDevOrTesting("unknown verb: %s %s", verb, lfsVerb) - return perm.AccessModeNone + return perm.AccessModeNone, false } func runServ(ctx context.Context, c *cli.Command) error { @@ -247,7 +249,10 @@ func runServ(ctx context.Context, c *cli.Command) error { } } - requestedMode := getAccessMode(verb, lfsVerb) + requestedMode, ok := getAccessMode(verb, lfsVerb) + if !ok { + return fail(ctx, "Unknown git command", "Unknown git command %s %s", verb, lfsVerb) + } results, extra := private.ServCommand(ctx, keyID, username, reponame, requestedMode, verb, lfsVerb) if extra.HasError() { diff --git a/cmd/serv_test.go b/cmd/serv_test.go new file mode 100644 index 0000000000..3bdcba1df3 --- /dev/null +++ b/cmd/serv_test.go @@ -0,0 +1,56 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package cmd + +import ( + "testing" + + "gitea.dev/models/perm" + "gitea.dev/modules/git" + + "github.com/stretchr/testify/assert" +) + +func TestGetAccessMode(t *testing.T) { + cases := []struct { + verb, lfsVerb string + expected perm.AccessMode + }{ + {git.CmdVerbUploadPack, "", perm.AccessModeRead}, + {git.CmdVerbUploadArchive, "", perm.AccessModeRead}, + {git.CmdVerbReceivePack, "", perm.AccessModeWrite}, + {git.CmdVerbLfsAuthenticate, git.CmdSubVerbLfsUpload, perm.AccessModeWrite}, + {git.CmdVerbLfsAuthenticate, git.CmdSubVerbLfsDownload, perm.AccessModeRead}, + {git.CmdVerbLfsTransfer, git.CmdSubVerbLfsUpload, perm.AccessModeWrite}, + {git.CmdVerbLfsTransfer, git.CmdSubVerbLfsDownload, perm.AccessModeRead}, + } + for _, tc := range cases { + t.Run(tc.verb+"/"+tc.lfsVerb, func(t *testing.T) { + mode, ok := getAccessMode(tc.verb, tc.lfsVerb) + assert.True(t, ok) + assert.Equal(t, tc.expected, mode) + }) + } +} + +// TestGetAccessModeUnknownVerb locks in the invariant that getAccessMode reports +// ok=false for unrecognised verbs and LFS sub-verbs, so runServ rejects them. An +// unknown verb has no valid access mode; if it were treated as AccessModeNone (0) +// it would pass the `userMode < mode` permission check in routers/private/serv.go +// and hand out valid LFS JWTs for any private repository. +func TestGetAccessModeUnknownVerb(t *testing.T) { + cases := []struct{ verb, lfsVerb string }{ + {git.CmdVerbLfsAuthenticate, ""}, + {git.CmdVerbLfsAuthenticate, "badverb"}, + {git.CmdVerbLfsTransfer, "badverb"}, + {"git-unknown-verb", ""}, + } + for _, tc := range cases { + t.Run(tc.verb+"/"+tc.lfsVerb, func(t *testing.T) { + mode, ok := getAccessMode(tc.verb, tc.lfsVerb) + assert.False(t, ok) + assert.Equal(t, perm.AccessModeNone, mode) + }) + } +}