From 0904421c8c471998740c7b5b69d789b53d3baa38 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 14 Mar 2022 09:02:36 +0800 Subject: [PATCH 1/6] Fix lfs bug --- modules/storage/local.go | 21 +++++++++++++++++++++ routers/web/repo/lfs.go | 13 +++++++++++++ 2 files changed, 34 insertions(+) diff --git a/modules/storage/local.go b/modules/storage/local.go index 022e6186d4e40..bad6e42d41860 100644 --- a/modules/storage/local.go +++ b/modules/storage/local.go @@ -6,6 +6,7 @@ package storage import ( "context" + "errors" "io" "net/url" "os" @@ -15,6 +16,8 @@ import ( "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 +62,18 @@ func NewLocalStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error // Open a file func (l *LocalStorage) Open(path string) (Object, error) { + if !l.isValid(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 !l.isValid(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 +117,19 @@ func (l *LocalStorage) Stat(path string) (os.FileInfo, error) { return os.Stat(filepath.Join(l.dir, path)) } +func (l *LocalStorage) isValid(path string) bool { + a, err := filepath.Abs(path) + if err != nil { + return false + } + return a == "/"+path +} + // Delete delete a file func (l *LocalStorage) Delete(path string) error { + if !l.isValid(path) { + return ErrLocalPathNotSupported + } p := filepath.Join(l.dir, path) return util.Remove(p) } diff --git a/routers/web/repo/lfs.go b/routers/web/repo/lfs.go index d2d62786fe7b3..36721b56a5f08 100644 --- a/routers/web/repo/lfs.go +++ b/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("LFSDelete", 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) From 5c73c639e2aa0f5f2b1040ac1f68ca8a3dd5d35a Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 14 Mar 2022 15:02:13 +0800 Subject: [PATCH 2/6] add more test --- modules/storage/local.go | 16 ++++++++------- modules/storage/local_test.go | 37 +++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 7 deletions(-) create mode 100644 modules/storage/local_test.go diff --git a/modules/storage/local.go b/modules/storage/local.go index bad6e42d41860..2eabcadc0d70b 100644 --- a/modules/storage/local.go +++ b/modules/storage/local.go @@ -7,10 +7,12 @@ package storage import ( "context" "errors" + "fmt" "io" "net/url" "os" "path/filepath" + "strings" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/util" @@ -62,7 +64,7 @@ func NewLocalStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error // Open a file func (l *LocalStorage) Open(path string) (Object, error) { - if !l.isValid(path) { + if !isLocalPathValid(path) { return nil, ErrLocalPathNotSupported } return os.Open(filepath.Join(l.dir, path)) @@ -70,7 +72,7 @@ func (l *LocalStorage) Open(path string) (Object, error) { // Save a file func (l *LocalStorage) Save(path string, r io.Reader, size int64) (int64, error) { - if !l.isValid(path) { + if !isLocalPathValid(path) { return 0, ErrLocalPathNotSupported } @@ -117,17 +119,17 @@ func (l *LocalStorage) Stat(path string) (os.FileInfo, error) { return os.Stat(filepath.Join(l.dir, path)) } -func (l *LocalStorage) isValid(path string) bool { - a, err := filepath.Abs(path) - if err != nil { +func isLocalPathValid(path string) bool { + a := filepath.Clean(path) + if strings.HasPrefix(a, fmt.Sprintf("..%c", filepath.Separator)) { return false } - return a == "/"+path + return a == path } // Delete delete a file func (l *LocalStorage) Delete(path string) error { - if !l.isValid(path) { + if !isLocalPathValid(path) { return ErrLocalPathNotSupported } p := filepath.Join(l.dir, path) diff --git a/modules/storage/local_test.go b/modules/storage/local_test.go new file mode 100644 index 0000000000000..2fca0c2589928 --- /dev/null +++ b/modules/storage/local_test.go @@ -0,0 +1,37 @@ +// 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) { + var kases = []struct{ + path string + valid bool + } { + { + "a/0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14", + true, + }, + { + "../a/0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14", + false, + }, + { + "b/../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)) + }) + } +} From 1609c0030e18b29cb199965ad3c06e4e30eb472d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 14 Mar 2022 15:04:48 +0800 Subject: [PATCH 3/6] Fix error string --- routers/web/repo/lfs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/web/repo/lfs.go b/routers/web/repo/lfs.go index 36721b56a5f08..c0f6b039d6116 100644 --- a/routers/web/repo/lfs.go +++ b/routers/web/repo/lfs.go @@ -256,7 +256,7 @@ func LFSFileGet(ctx *context.Context) { p := lfs.Pointer{Oid: oid} if !p.IsValid() { - ctx.NotFound("LFSDelete", nil) + ctx.NotFound("LFSFileGet", nil) return } From 989d0ef7bb0cbdd627102dc9f7078d3e46926b02 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 14 Mar 2022 17:34:25 +0800 Subject: [PATCH 4/6] Fix fmt --- modules/storage/local_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/storage/local_test.go b/modules/storage/local_test.go index 2fca0c2589928..962e52cbbda3f 100644 --- a/modules/storage/local_test.go +++ b/modules/storage/local_test.go @@ -11,10 +11,10 @@ import ( ) func TestLocalPathIsValid(t *testing.T) { - var kases = []struct{ - path string + kases := []struct { + path string valid bool - } { + }{ { "a/0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14", true, From a465347ea9b8ba9a07af7ed526f5268cee044abd Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 14 Mar 2022 21:13:18 +0800 Subject: [PATCH 5/6] Fix test --- modules/storage/local.go | 3 +-- modules/storage/local_test.go | 4 ++++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/modules/storage/local.go b/modules/storage/local.go index 2eabcadc0d70b..243347d58984b 100644 --- a/modules/storage/local.go +++ b/modules/storage/local.go @@ -7,7 +7,6 @@ package storage import ( "context" "errors" - "fmt" "io" "net/url" "os" @@ -121,7 +120,7 @@ func (l *LocalStorage) Stat(path string) (os.FileInfo, error) { func isLocalPathValid(path string) bool { a := filepath.Clean(path) - if strings.HasPrefix(a, fmt.Sprintf("..%c", filepath.Separator)) { + if strings.HasPrefix(a, "../") || strings.HasPrefix(a, "..\\") { return false } return a == path diff --git a/modules/storage/local_test.go b/modules/storage/local_test.go index 962e52cbbda3f..ffd8d3456a10c 100644 --- a/modules/storage/local_test.go +++ b/modules/storage/local_test.go @@ -27,6 +27,10 @@ func TestLocalPathIsValid(t *testing.T) { "b/../a/0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14", false, }, + { + "..\\a/0/a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a14", + false, + }, } for _, k := range kases { From ab8c6968997e821d00e13d2bb952c58324e60956 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 14 Mar 2022 21:18:33 +0800 Subject: [PATCH 6/6] Fix test --- modules/storage/local.go | 7 ++++--- modules/storage/local_test.go | 4 ++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/modules/storage/local.go b/modules/storage/local.go index 243347d58984b..8d9aa603d09ec 100644 --- a/modules/storage/local.go +++ b/modules/storage/local.go @@ -10,6 +10,7 @@ import ( "io" "net/url" "os" + "path" "path/filepath" "strings" @@ -118,12 +119,12 @@ func (l *LocalStorage) Stat(path string) (os.FileInfo, error) { return os.Stat(filepath.Join(l.dir, path)) } -func isLocalPathValid(path string) bool { - a := filepath.Clean(path) +func isLocalPathValid(p string) bool { + a := path.Clean(p) if strings.HasPrefix(a, "../") || strings.HasPrefix(a, "..\\") { return false } - return a == path + return a == p } // Delete delete a file diff --git a/modules/storage/local_test.go b/modules/storage/local_test.go index ffd8d3456a10c..8714f37f0da65 100644 --- a/modules/storage/local_test.go +++ b/modules/storage/local_test.go @@ -23,6 +23,10 @@ func TestLocalPathIsValid(t *testing.T) { "../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,