fix(lfs): reject unknown SSH LFS sub-verbs to prevent auth bypass - #38008
Conversation
An authenticated SSH user could pass a malformed sub-verb (e.g. `git-lfs-authenticate <repo> 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.
5e548db to
2bc979d
Compare
Address review feedback from @lunny: move the LFS sub-verb allow-list next to IsAllowedVerbForServeLfs in modules/git/cmdverb.go and use it from runServ.
wxiaoguang
left a comment
There was a problem hiding this comment.
The fix and test is not right.
It should not depends on the "panic" or "mode=none" behavior
@wxiaoguang fixed |
I saw... |
|
As a quick fix we can go for it including backport solution by wxiaoguang . But complete rewrite need more changes and more time. Approving it as of now until we are not going to rewrite it. |
|
@bircni Let me know if you are planning a backport of this. |
maybe we should write an issue? |
@bircni IMO we can explain the issue in more detail for re-write. It took me significant time to access the problem, actually. |
|
My approval is done. Good to merge the backport as well. |
This PR contains the following updates: | Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) | |---|---|---|---| | [code.gitea.io/gitea](https://github.com/go-gitea/gitea) | `v1.26.2` → `v1.26.3` |  |  | --- ### Release Notes <details> <summary>go-gitea/gitea (code.gitea.io/gitea)</summary> ### [`v1.26.3`](https://github.com/go-gitea/gitea/releases/tag/v1.26.3) [Compare Source](go-gitea/gitea@v1.26.2...v1.26.3) - BREAKING - fix(actions)!: require merged PR to bypass fork PR approval gate ([#​38010](go-gitea/gitea#38010)) ([#​38041](go-gitea/gitea#38041)) - SECURITY - fix(hostmatcher): patch incorrect private list ([#​38170](go-gitea/gitea#38170)) ([#​38173](go-gitea/gitea#38173)) - fix: Various security fixes ([#​38103](go-gitea/gitea#38103)) ([#​38151](go-gitea/gitea#38151)) - fix: Various sec fixes ([#​38108](go-gitea/gitea#38108)) ([#​38147](go-gitea/gitea#38147)) - fix: allow git clone of private repos with anonymous code access ([#​38074](go-gitea/gitea#38074)) ([#​38146](go-gitea/gitea#38146)) - fix(auth): ignore stale OIDC external login links to organizations ([#​37875](go-gitea/gitea#37875)) ([#​38141](go-gitea/gitea#38141)) - fix(hostmatcher): block reserved IP ranges from external/private filters ([#​38039](go-gitea/gitea#38039)) ([#​38059](go-gitea/gitea#38059)) - fix(lfs): require Code-unit access for cross-repo LFS object reuse ([#​38006](go-gitea/gitea#38006)) ([#​38050](go-gitea/gitea#38050)) - fix(lfs): reject unknown SSH LFS sub-verbs to prevent auth bypass ([#​38008](go-gitea/gitea#38008)) ([#​38015](go-gitea/gitea#38015)) - fix: bound CODEOWNERS regex match time ([#​38011](go-gitea/gitea#38011)) ([#​38025](go-gitea/gitea#38025)) - fix: bound debian ParseControlFile to a single control stanza ([#​38044](go-gitea/gitea#38044)) ([#​38055](go-gitea/gitea#38055)) - fix(deps): update module golang.org/x/net to v0.55.0 \[security] ([#​37813](go-gitea/gitea#37813)) ([#​37829](go-gitea/gitea#37829)) - API - feat(api): add Link header in ListForks ([#​38052](go-gitea/gitea#38052)) ([#​38063](go-gitea/gitea#38063)) - BUGFIXES - fix: Fix the panic when ssh remote lfs endpoint parsing failure ([#​38026](go-gitea/gitea#38026)) ([#​38158](go-gitea/gitea#38158)) - fix(api): nil pointer panic when filtering tracked times by a non-existent user ([#​38112](go-gitea/gitea#38112)) ([#​38115](go-gitea/gitea#38115)) - fix: keep literal "false" value displayed in workflow\_dispatch choice dropdowns ([#​38080](go-gitea/gitea#38080)) ([#​38096](go-gitea/gitea#38096)) - fix: parse HEAD ref ([#​38119](go-gitea/gitea#38119)) - fix: git cmd ([#​38084](go-gitea/gitea#38084)) ([#​38087](go-gitea/gitea#38087)) - fix(releases): generate notes for initial tag ([#​37697](go-gitea/gitea#37697)) ([#​37986](go-gitea/gitea#37986)) - fix(actions): return 404 when job log blob is missing ([#​38003](go-gitea/gitea#38003)) ([#​38004](go-gitea/gitea#38004)) - fix(actions): exclude `workflow_call` from workflow trigger detection ([#​37894](go-gitea/gitea#37894)) ([#​37899](go-gitea/gitea#37899)) - fix(actions): keep action run title clickable when commit subject is a URL ([#​37867](go-gitea/gitea#37867)) ([#​37898](go-gitea/gitea#37898)) - fix(actions): reject workflow\_dispatch for workflows without that trigger ([#​37660](go-gitea/gitea#37660)) ([#​37895](go-gitea/gitea#37895)) - fix(actions): ack re-sent `UpdateLog` finalize idempotently ([#​37885](go-gitea/gitea#37885)) ([#​37892](go-gitea/gitea#37892)) - fix: http content file render ([#​37850](go-gitea/gitea#37850)) ([#​37856](go-gitea/gitea#37856)) - fix(issues): clear stale ReviewTypeRequest when submitting pending review ([#​37809](go-gitea/gitea#37809)) ([#​37815](go-gitea/gitea#37815)) - fix: Fix issue target branch selection for non-collaborators ([#​36916](go-gitea/gitea#36916)) ([#​38164](go-gitea/gitea#38164)) - BUILD - fix(deps): update `@playwright/test` to 1.60.0 ([#​38144](go-gitea/gitea#38144)) - ci: add `tools/ci-tools.ts` for the PR labeler workflow ([#​37831](go-gitea/gitea#37831)) - fix(build): swagger css import ([#​37801](go-gitea/gitea#37801)) ([#​37803](go-gitea/gitea#37803)) Instances on **[Gitea Cloud](https://cloud.gitea.com)** will be automatically upgraded to this version during the specified maintenance window. </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMDEuMSIsInVwZGF0ZWRJblZlciI6IjQzLjEwMS4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJ0eXBlL3BhdGNoIl19--> Reviewed-on: https://git.erwanleboucher.dev/eleboucher/apoci/pulls/89
This PR contains the following updates: | Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) | |---|---|---|---| | [code.gitea.io/gitea](https://github.com/go-gitea/gitea) | `v1.26.2` → `v1.26.4` |  |  | --- ### Release Notes <details> <summary>go-gitea/gitea (code.gitea.io/gitea)</summary> ### [`v1.26.4`](https://github.com/go-gitea/gitea/releases/tag/v1.26.4) [Compare Source](go-gitea/gitea@v1.26.3...v1.26.4) - SECURITY - fix(auth): do not auto-reactivate disabled users on OAuth2 callback ([#​38009](go-gitea/gitea#38009)) ([#​38183](go-gitea/gitea#38183)) - BUGFIXES - fix: walk git log context error handling ([#​38182](go-gitea/gitea#38182)) ([#​38185](go-gitea/gitea#38185)) Instances on **[Gitea Cloud](https://cloud.gitea.com)** will be automatically upgraded to this version during the specified maintenance window. ### [`v1.26.3`](https://github.com/go-gitea/gitea/releases/tag/v1.26.3) [Compare Source](go-gitea/gitea@v1.26.2...v1.26.3) > \[!WARNING] > **Please upgrade to 1.26.4 directly.** A regression in this release can cause **"context deadline exceeded"** errors when opening any repository's code pages ([#​38177](go-gitea/gitea#38177)). Please hold off on upgrading until a fix is released. - BREAKING - fix(actions)!: require merged PR to bypass fork PR approval gate ([#​38010](go-gitea/gitea#38010)) ([#​38041](go-gitea/gitea#38041)) - SECURITY - fix(hostmatcher): patch incorrect private list ([#​38170](go-gitea/gitea#38170)) ([#​38173](go-gitea/gitea#38173)) - fix: Various security fixes ([#​38103](go-gitea/gitea#38103)) ([#​38151](go-gitea/gitea#38151)) - fix: Various sec fixes ([#​38108](go-gitea/gitea#38108)) ([#​38147](go-gitea/gitea#38147)) - fix: allow git clone of private repos with anonymous code access ([#​38074](go-gitea/gitea#38074)) ([#​38146](go-gitea/gitea#38146)) - fix(auth): ignore stale OIDC external login links to organizations ([#​37875](go-gitea/gitea#37875)) ([#​38141](go-gitea/gitea#38141)) - fix(hostmatcher): block reserved IP ranges from external/private filters ([#​38039](go-gitea/gitea#38039)) ([#​38059](go-gitea/gitea#38059)) - fix(lfs): require Code-unit access for cross-repo LFS object reuse ([#​38006](go-gitea/gitea#38006)) ([#​38050](go-gitea/gitea#38050)) - fix(lfs): reject unknown SSH LFS sub-verbs to prevent auth bypass ([#​38008](go-gitea/gitea#38008)) ([#​38015](go-gitea/gitea#38015)) - fix: bound CODEOWNERS regex match time ([#​38011](go-gitea/gitea#38011)) ([#​38025](go-gitea/gitea#38025)) - fix: bound debian ParseControlFile to a single control stanza ([#​38044](go-gitea/gitea#38044)) ([#​38055](go-gitea/gitea#38055)) - fix(deps): update module golang.org/x/net to v0.55.0 \[security] ([#​37813](go-gitea/gitea#37813)) ([#​37829](go-gitea/gitea#37829)) - API - feat(api): add Link header in ListForks ([#​38052](go-gitea/gitea#38052)) ([#​38063](go-gitea/gitea#38063)) - BUGFIXES - fix: Fix the panic when ssh remote lfs endpoint parsing failure ([#​38026](go-gitea/gitea#38026)) ([#​38158](go-gitea/gitea#38158)) - fix(api): nil pointer panic when filtering tracked times by a non-existent user ([#​38112](go-gitea/gitea#38112)) ([#​38115](go-gitea/gitea#38115)) - fix: keep literal "false" value displayed in workflow\_dispatch choice dropdowns ([#​38080](go-gitea/gitea#38080)) ([#​38096](go-gitea/gitea#38096)) - fix: parse HEAD ref ([#​38119](go-gitea/gitea#38119)) - fix: git cmd ([#​38084](go-gitea/gitea#38084)) ([#​38087](go-gitea/gitea#38087)) - fix(releases): generate notes for initial tag ([#​37697](go-gitea/gitea#37697)) ([#​37986](go-gitea/gitea#37986)) - fix(actions): return 404 when job log blob is missing ([#​38003](go-gitea/gitea#38003)) ([#​38004](go-gitea/gitea#38004)) - fix(actions): exclude `workflow_call` from workflow trigger detection ([#​37894](go-gitea/gitea#37894)) ([#​37899](go-gitea/gitea#37899)) - fix(actions): keep action run title clickable when commit subject is a URL ([#​37867](go-gitea/gitea#37867)) ([#​37898](go-gitea/gitea#37898)) - fix(actions): reject workflow\_dispatch for workflows without that trigger ([#​37660](go-gitea/gitea#37660)) ([#​37895](go-gitea/gitea#37895)) - fix(actions): ack re-sent `UpdateLog` finalize idempotently ([#​37885](go-gitea/gitea#37885)) ([#​37892](go-gitea/gitea#37892)) - fix: http content file render ([#​37850](go-gitea/gitea#37850)) ([#​37856](go-gitea/gitea#37856)) - fix(issues): clear stale ReviewTypeRequest when submitting pending review ([#​37809](go-gitea/gitea#37809)) ([#​37815](go-gitea/gitea#37815)) - fix: Fix issue target branch selection for non-collaborators ([#​36916](go-gitea/gitea#36916)) ([#​38164](go-gitea/gitea#38164)) - BUILD - fix(deps): update `@playwright/test` to 1.60.0 ([#​38144](go-gitea/gitea#38144)) - ci: add `tools/ci-tools.ts` for the PR labeler workflow ([#​37831](go-gitea/gitea#37831)) - fix(build): swagger css import ([#​37801](go-gitea/gitea#37801)) ([#​37803](go-gitea/gitea#37803)) Instances on **[Gitea Cloud](https://cloud.gitea.com)** will be automatically upgraded to this version during the specified maintenance window. </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMDEuMSIsInVwZGF0ZWRJblZlciI6IjQzLjEwMS4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJ0eXBlL3BhdGNoIl19--> Reviewed-on: https://git.erwanleboucher.dev/eleboucher/apoci/pulls/95
…-gitea#38008) An authenticated SSH user could pass a malformed sub-verb (e.g. `git-lfs-authenticate <repo> 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.

An authenticated SSH user could pass a malformed sub-verb (e.g.
git-lfs-authenticate <repo> badverb) so getAccessMode falls through to AccessModeNone (0). The permission check in routers/private/serv.go then evaluatesuserMode < 0which 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.