diff --git a/models/git/lfs_lock.go b/models/git/lfs_lock.go index 25480f3f9..178fa72f0 100644 --- a/models/git/lfs_lock.go +++ b/models/git/lfs_lock.go @@ -6,7 +6,6 @@ package git import ( "context" "fmt" - "path" "strings" "time" @@ -17,6 +16,7 @@ import ( "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" ) // LFSLock represents a git lfs lock of repository. @@ -34,11 +34,7 @@ func init() { // BeforeInsert is invoked from XORM before inserting an object of this type. func (l *LFSLock) BeforeInsert() { - l.Path = cleanPath(l.Path) -} - -func cleanPath(p string) string { - return path.Clean("/" + p)[1:] + l.Path = util.CleanPath(l.Path) } // CreateLFSLock creates a new lock. @@ -53,7 +49,7 @@ func CreateLFSLock(ctx context.Context, repo *repo_model.Repository, lock *LFSLo return nil, err } - lock.Path = cleanPath(lock.Path) + lock.Path = util.CleanPath(lock.Path) lock.RepoID = repo.ID l, err := GetLFSLock(dbCtx, repo, lock.Path) @@ -73,7 +69,7 @@ func CreateLFSLock(ctx context.Context, repo *repo_model.Repository, lock *LFSLo // GetLFSLock returns release by given path. func GetLFSLock(ctx context.Context, repo *repo_model.Repository, path string) (*LFSLock, error) { - path = cleanPath(path) + path = util.CleanPath(path) rel := &LFSLock{RepoID: repo.ID} has, err := db.GetEngine(ctx).Where("lower(path) = ?", strings.ToLower(path)).Get(rel) if err != nil { diff --git a/modules/options/base.go b/modules/options/base.go index 3c140f643..e83e8df5d 100644 --- a/modules/options/base.go +++ b/modules/options/base.go @@ -16,27 +16,27 @@ import ( // Locale reads the content of a specific locale from static/bindata or custom path. func Locale(name string) ([]byte, error) { - return fileFromDir(path.Join("locale", path.Clean("/"+name))) + return fileFromDir(path.Join("locale", util.CleanPath(name))) } // Readme reads the content of a specific readme from static/bindata or custom path. func Readme(name string) ([]byte, error) { - return fileFromDir(path.Join("readme", path.Clean("/"+name))) + return fileFromDir(path.Join("readme", util.CleanPath(name))) } // Gitignore reads the content of a gitignore locale from static/bindata or custom path. func Gitignore(name string) ([]byte, error) { - return fileFromDir(path.Join("gitignore", path.Clean("/"+name))) + return fileFromDir(path.Join("gitignore", util.CleanPath(name))) } // License reads the content of a specific license from static/bindata or custom path. func License(name string) ([]byte, error) { - return fileFromDir(path.Join("license", path.Clean("/"+name))) + return fileFromDir(path.Join("license", util.CleanPath(name))) } // Labels reads the content of a specific labels from static/bindata or custom path. func Labels(name string) ([]byte, error) { - return fileFromDir(path.Join("label", path.Clean("/"+name))) + return fileFromDir(path.Join("label", util.CleanPath(name))) } // WalkLocales reads the content of a specific locale diff --git a/modules/public/public.go b/modules/public/public.go index 42026f9b1..e1d60d89e 100644 --- a/modules/public/public.go +++ b/modules/public/public.go @@ -6,7 +6,6 @@ package public import ( "net/http" "os" - "path" "path/filepath" "strings" @@ -14,6 +13,7 @@ import ( "code.gitea.io/gitea/modules/httpcache" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" ) // Options represents the available options to configure the handler. @@ -103,7 +103,7 @@ func setWellKnownContentType(w http.ResponseWriter, file string) { func (opts *Options) handle(w http.ResponseWriter, req *http.Request, fs http.FileSystem, file string) bool { // use clean to keep the file is a valid path with no . or .. - f, err := fs.Open(path.Clean(file)) + f, err := fs.Open(util.CleanPath(file)) if err != nil { if os.IsNotExist(err) { return false diff --git a/modules/storage/local.go b/modules/storage/local.go index a6a9d54a8..05bf1fb28 100644 --- a/modules/storage/local.go +++ b/modules/storage/local.go @@ -8,7 +8,6 @@ import ( "io" "net/url" "os" - "path" "path/filepath" "strings" @@ -59,7 +58,7 @@ func NewLocalStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error } func (l *LocalStorage) buildLocalPath(p string) string { - return filepath.Join(l.dir, path.Clean("/" + strings.ReplaceAll(p, "\\", "/"))[1:]) + return filepath.Join(l.dir, util.CleanPath(strings.ReplaceAll(p, "\\", "/"))) } // Open a file diff --git a/modules/storage/minio.go b/modules/storage/minio.go index c427d8d7e..24da14b63 100644 --- a/modules/storage/minio.go +++ b/modules/storage/minio.go @@ -15,6 +15,7 @@ import ( "time" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" "github.com/minio/minio-go/v7" "github.com/minio/minio-go/v7/pkg/credentials" @@ -120,7 +121,7 @@ func NewMinioStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error } func (m *MinioStorage) buildMinioPath(p string) string { - return strings.TrimPrefix(path.Join(m.basePath, path.Clean("/" + strings.ReplaceAll(p, "\\", "/"))[1:]), "/") + return strings.TrimPrefix(path.Join(m.basePath, util.CleanPath(strings.ReplaceAll(p, "\\", "/"))), "/") } // Open open a file diff --git a/modules/util/path.go b/modules/util/path.go index 74acb7a85..5aa9e15f5 100644 --- a/modules/util/path.go +++ b/modules/util/path.go @@ -14,6 +14,14 @@ import ( "strings" ) +// CleanPath ensure to clean the path +func CleanPath(p string) string { + if strings.HasPrefix(p, "/") { + return path.Clean(p) + } + return path.Clean("/" + p)[1:] +} + // EnsureAbsolutePath ensure that a path is absolute, making it // relative to absoluteBase if necessary func EnsureAbsolutePath(path, absoluteBase string) string { diff --git a/modules/util/path_test.go b/modules/util/path_test.go index 93f4f67cf..2f020f924 100644 --- a/modules/util/path_test.go +++ b/modules/util/path_test.go @@ -136,3 +136,15 @@ func TestMisc_IsReadmeFileName(t *testing.T) { assert.Equal(t, testCase.idx, idx) } } + +func TestCleanPath(t *testing.T) { + cases := map[string]string{ + "../../test": "test", + "/test": "/test", + "/../test": "/test", + } + + for k, v := range cases { + assert.Equal(t, v, CleanPath(k)) + } +} diff --git a/routers/web/base.go b/routers/web/base.go index b0d8a7c3f..d0135eac7 100644 --- a/routers/web/base.go +++ b/routers/web/base.go @@ -19,6 +19,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/templates" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web/middleware" "code.gitea.io/gitea/modules/web/routing" "code.gitea.io/gitea/services/auth" @@ -44,7 +45,7 @@ func storageHandler(storageSetting setting.Storage, prefix string, objStore stor routing.UpdateFuncInfo(req.Context(), funcInfo) rPath := strings.TrimPrefix(req.URL.Path, "/"+prefix+"/") - rPath = path.Clean("/" + strings.ReplaceAll(rPath, "\\", "/"))[1:] + rPath = util.CleanPath(strings.ReplaceAll(rPath, "\\", "/")) u, err := objStore.URL(rPath, path.Base(rPath)) if err != nil { @@ -80,7 +81,7 @@ func storageHandler(storageSetting setting.Storage, prefix string, objStore stor routing.UpdateFuncInfo(req.Context(), funcInfo) rPath := strings.TrimPrefix(req.URL.Path, "/"+prefix+"/") - rPath = path.Clean("/" + strings.ReplaceAll(rPath, "\\", "/"))[1:] + rPath = util.CleanPath(strings.ReplaceAll(rPath, "\\", "/")) if rPath == "" { http.Error(w, "file not found", http.StatusNotFound) return diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index e5ba4ad2c..4f208098e 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -726,7 +726,7 @@ func UploadFilePost(ctx *context.Context) { func cleanUploadFileName(name string) string { // Rebase the filename - name = strings.Trim(path.Clean("/"+name), "/") + name = strings.Trim(util.CleanPath(name), "/") // Git disallows any filenames to have a .git directory in them. for _, part := range strings.Split(name, "/") { if strings.ToLower(part) == ".git" { diff --git a/routers/web/repo/lfs.go b/routers/web/repo/lfs.go index 869a69c37..43f552798 100644 --- a/routers/web/repo/lfs.go +++ b/routers/web/repo/lfs.go @@ -207,7 +207,7 @@ func LFSLockFile(ctx *context.Context) { ctx.Redirect(ctx.Repo.RepoLink + "/settings/lfs/locks") return } - lockPath = path.Clean("/" + lockPath)[1:] + lockPath = util.CleanPath(lockPath) if len(lockPath) == 0 { ctx.Flash.Error(ctx.Tr("repo.settings.lfs_invalid_locking_path", originalPath)) ctx.Redirect(ctx.Repo.RepoLink + "/settings/lfs/locks") diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go index 8b259a362..ca961524d 100644 --- a/services/migrations/gitea_uploader.go +++ b/services/migrations/gitea_uploader.go @@ -9,7 +9,6 @@ import ( "fmt" "io" "os" - "path" "path/filepath" "strconv" "strings" @@ -30,6 +29,7 @@ import ( "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/uri" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/services/pull" "github.com/google/uuid" @@ -866,7 +866,7 @@ func (g *GiteaLocalUploader) CreateReviews(reviews ...*base.Review) error { } // SECURITY: The TreePath must be cleaned! - comment.TreePath = path.Clean("/" + comment.TreePath)[1:] + comment.TreePath = util.CleanPath(comment.TreePath) var patch string reader, writer := io.Pipe() diff --git a/services/packages/container/blob_uploader.go b/services/packages/container/blob_uploader.go index ba92b0507..860672587 100644 --- a/services/packages/container/blob_uploader.go +++ b/services/packages/container/blob_uploader.go @@ -8,13 +8,13 @@ import ( "errors" "io" "os" - "path" "path/filepath" "strings" packages_model "code.gitea.io/gitea/models/packages" packages_module "code.gitea.io/gitea/modules/packages" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" ) var ( @@ -33,7 +33,7 @@ type BlobUploader struct { } func buildFilePath(id string) string { - return filepath.Join(setting.Packages.ChunkedUploadPath, path.Clean("/" + strings.ReplaceAll(id, "\\", "/"))[1:]) + return filepath.Join(setting.Packages.ChunkedUploadPath, util.CleanPath(strings.ReplaceAll(id, "\\", "/"))) } // NewBlobUploader creates a new blob uploader for the given id diff --git a/services/repository/files/file.go b/services/repository/files/file.go index 2bac4372d..7939491ae 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -7,7 +7,6 @@ import ( "context" "fmt" "net/url" - "path" "strings" "time" @@ -15,6 +14,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" ) // GetFileResponseFromCommit Constructs a FileResponse from a Commit object @@ -129,7 +129,7 @@ func GetAuthorAndCommitterUsers(author, committer *IdentityOptions, doer *user_m // CleanUploadFileName Trims a filename and returns empty string if it is a .git directory func CleanUploadFileName(name string) string { // Rebase the filename - name = strings.Trim(path.Clean("/"+name), "/") + name = strings.Trim(util.CleanPath(name), "/") // Git disallows any filenames to have a .git directory in them. for _, part := range strings.Split(name, "/") { if strings.ToLower(part) == ".git" {