mirror of
https://github.com/go-gitea/gitea.git
synced 2026-07-10 03:10:00 +00:00
Backport #38322 by @bircni The LFS batch and upload handlers linked an object that already existed in the content store but was not linked to the current repo whenever the token's user could access it in another repo. Deploy-key tokens carry the repo owner's identity, so a single-repo write deploy key could link and then download objects from any repo the owner can see. This drops the cross-repo access check: the batch handler now makes the client upload (hash-verified) any object not yet linked to the repo, and the upload handler skips proof of possession only when the object is already linked to the current repo. Co-authored-by: bircni <bircni@icloud.com>
This commit is contained in:
@@ -254,6 +254,17 @@ func BatchHandler(ctx *context.Context) {
|
||||
|
||||
var responseObject *lfs_module.ObjectResponse
|
||||
if isUpload {
|
||||
if exists && meta == nil {
|
||||
// The object exists in the content store but is not linked to this
|
||||
// repo. Do not auto-link it based on cross-repo access: the token
|
||||
// only authorizes this repo, and for deploy keys ctx.Doer is the
|
||||
// repo owner, so trusting it here would let a single-repo key pull
|
||||
// objects from any repo the owner can see. Require proof of
|
||||
// possession by making the client upload the (hash-verified) bytes,
|
||||
// and treat it as a new upload for size-limit enforcement below.
|
||||
exists = false
|
||||
}
|
||||
|
||||
var err *lfs_module.ObjectError
|
||||
if !exists && setting.LFS.MaxFileSize > 0 && p.Size > setting.LFS.MaxFileSize {
|
||||
err = &lfs_module.ObjectError{
|
||||
@@ -262,25 +273,6 @@ func BatchHandler(ctx *context.Context) {
|
||||
}
|
||||
}
|
||||
|
||||
if exists && meta == nil {
|
||||
accessible, err := git_model.LFSObjectAccessible(ctx, ctx.Doer, p.Oid)
|
||||
if err != nil {
|
||||
log.Error("Unable to check if LFS MetaObject [%s] is accessible. Error: %v", p.Oid, err)
|
||||
writeStatus(ctx, http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
if accessible {
|
||||
_, err := git_model.NewLFSMetaObject(ctx, repository.ID, p)
|
||||
if err != nil {
|
||||
log.Error("Unable to create LFS MetaObject [%s] for %s/%s. Error: %v", p.Oid, rc.User, rc.Repo, err)
|
||||
writeStatus(ctx, http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
} else {
|
||||
exists = false
|
||||
}
|
||||
}
|
||||
|
||||
responseObject = buildObjectResponse(rc, p, false, !exists, err)
|
||||
} else {
|
||||
var err *lfs_module.ObjectError
|
||||
@@ -337,13 +329,17 @@ func UploadHandler(ctx *context.Context) {
|
||||
|
||||
uploadOrVerify := func() error {
|
||||
if exists {
|
||||
accessible, err := git_model.LFSObjectAccessible(ctx, ctx.Doer, p.Oid)
|
||||
if err != nil {
|
||||
log.Error("Unable to check if LFS MetaObject [%s] is accessible. Error: %v", p.Oid, err)
|
||||
// The bytes already exist in the content store. Only skip proof of
|
||||
// possession when the object is already linked to *this* repo; never
|
||||
// trust cross-repo access (ctx.Doer is the repo owner for deploy keys),
|
||||
// which would let a caller link an object it cannot produce.
|
||||
meta, err := git_model.GetLFSMetaObjectByOid(ctx, repository.ID, p.Oid)
|
||||
if err != nil && err != git_model.ErrLFSObjectNotExist {
|
||||
log.Error("Unable to get LFS MetaObject [%s]. Error: %v", p.Oid, err)
|
||||
return err
|
||||
}
|
||||
if !accessible {
|
||||
// The file exists but the user has no access to it.
|
||||
if meta == nil {
|
||||
// The file exists but is not linked to this repo.
|
||||
// The upload gets verified by hashing and size comparison to prove access to it.
|
||||
hash := sha256.New()
|
||||
written, err := io.Copy(hash, ctx.Req.Body)
|
||||
|
||||
@@ -240,9 +240,14 @@ func TestAPILFSBatch(t *testing.T) {
|
||||
assert.Equal(t, "Size must be less than or equal to 2", br.Objects[0].Error.Message)
|
||||
})
|
||||
|
||||
t.Run("AddMeta", func(t *testing.T) {
|
||||
t.Run("CrossRepoObjectRequiresUpload", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
|
||||
// An object whose bytes already exist in the store but which is not
|
||||
// linked to this repo must not be silently linked, even when the
|
||||
// caller can access it in another repo. Auto-linking let a deploy key
|
||||
// (whose token carries the repo owner's identity) exfiltrate objects
|
||||
// across repos without proving possession. The client must upload.
|
||||
p := lfs.Pointer{Oid: "05eeb4eb5be71f2dd291ca39157d6d9effd7d1ea19cbdc8a99411fe2a8f26a00", Size: 6}
|
||||
|
||||
contentStore := lfs.NewContentStore()
|
||||
@@ -250,6 +255,7 @@ func TestAPILFSBatch(t *testing.T) {
|
||||
assert.NoError(t, err)
|
||||
assert.True(t, exist)
|
||||
|
||||
// The object is linked to another repo owned by the same user.
|
||||
repo2 := createLFSTestRepository(t, "lfs-batch2-repo")
|
||||
storeObjectInRepo(t, repo2.ID, "dummy0")
|
||||
|
||||
@@ -266,11 +272,13 @@ func TestAPILFSBatch(t *testing.T) {
|
||||
br := decodeResponse(t, resp.Body)
|
||||
assert.Len(t, br.Objects, 1)
|
||||
assert.Nil(t, br.Objects[0].Error)
|
||||
assert.Empty(t, br.Objects[0].Actions)
|
||||
// The client is told to upload instead of the object being linked.
|
||||
assert.Contains(t, br.Objects[0].Actions, "upload")
|
||||
|
||||
// No meta object may have been created for this repo.
|
||||
meta, err = git_model.GetLFSMetaObjectByOid(t.Context(), repo.ID, p.Oid)
|
||||
assert.NoError(t, err)
|
||||
assert.NotNil(t, meta)
|
||||
assert.Nil(t, meta)
|
||||
assert.Equal(t, git_model.ErrLFSObjectNotExist, err)
|
||||
|
||||
// Cleanup
|
||||
err = contentStore.Delete(p.RelativePath())
|
||||
|
||||
Reference in New Issue
Block a user