Fix lfs bug (#19072) * Fix lfs bug

Changes

3 changed files (+81/-0)

• modules/storage/local.go
  • @@ -6,15 +6,20 @@ package storage
    import ( "context" + "errors" "io" "net/url" "os" + "path" "path/filepath" + "strings" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/util" ) + // ErrLocalPathNotSupported represents an error that path is not supported + var ErrLocalPathNotSupported = errors.New("local path is not supported") var _ ObjectStorage = &LocalStorage{} // LocalStorageType is the type descriptor for local storage
  • @@ -59,11 +64,18 @@ func NewLocalStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error
    // Open a file func (l *LocalStorage) Open(path string) (Object, error) { + if !isLocalPathValid(path) { + return nil, ErrLocalPathNotSupported + } return os.Open(filepath.Join(l.dir, path)) } // Save a file func (l *LocalStorage) Save(path string, r io.Reader, size int64) (int64, error) { + if !isLocalPathValid(path) { + return 0, ErrLocalPathNotSupported + } + p := filepath.Join(l.dir, path) if err := os.MkdirAll(filepath.Dir(p), os.ModePerm); err != nil { return 0, err
  • @@ -107,8 +119,19 @@ func (l *LocalStorage) Stat(path string) (os.FileInfo, error) {
    return os.Stat(filepath.Join(l.dir, path)) } + func isLocalPathValid(p string) bool { + a := path.Clean(p) + if strings.HasPrefix(a, "../") || strings.HasPrefix(a, "..\\") { + return false + } + return a == p + } + // Delete delete a file func (l *LocalStorage) Delete(path string) error { + if !isLocalPathValid(path) { + return ErrLocalPathNotSupported + } p := filepath.Join(l.dir, path) return util.Remove(p) }
• modules/storage/local_test.go (new)
  • @@ -0,0 +1,45 @@
    + // Copyright 2022 The Gitea Authors. All rights reserved. + // Use of this source code is governed by a MIT-style + // license that can be found in the LICENSE file. + + package storage + + import ( + "testing" + + "github.com/stretchr/testify/assert" + ) + + func TestLocalPathIsValid(t *testing.T) { + kases := []struct { + path string + valid bool + }{ + { + "a/0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14", + true, + }, + { + "../a/0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14", + false, + }, + { + "a\\0\\a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14", + true, + }, + { + "b/../a/0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14", + false, + }, + { + "..\\a/0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14", + false, + }, + } + + for _, k := range kases { + t.Run(k.path, func(t *testing.T) { + assert.EqualValues(t, k.valid, isLocalPathValid(k.path)) + }) + } + }
• routers/web/repo/lfs.go
  • @@ -253,6 +253,13 @@ func LFSFileGet(ctx *context.Context) {
    } ctx.Data["LFSFilesLink"] = ctx.Repo.RepoLink + "/settings/lfs" oid := ctx.Params("oid") + + p := lfs.Pointer{Oid: oid} + if !p.IsValid() { + ctx.NotFound("LFSFileGet", nil) + return + } + ctx.Data["Title"] = oid ctx.Data["PageIsSettingsLFS"] = true meta, err := models.GetLFSMetaObjectByOid(ctx.Repo.Repository.ID, oid)
  • @@ -343,6 +350,12 @@ func LFSDelete(ctx *context.Context) {
    return } oid := ctx.Params("oid") + p := lfs.Pointer{Oid: oid} + if !p.IsValid() { + ctx.NotFound("LFSDelete", nil) + return + } + count, err := models.RemoveLFSMetaObjectByOid(ctx.Repo.Repository.ID, oid) if err != nil { ctx.ServerError("LFSDelete", err)