Use CleanPath instead of path.Clean (#23371)

As title.
This commit is contained in:
Lunny Xiao 2023-03-08 20:17:39 +08:00 committed by GitHub
parent 090e753923
commit b116418f05
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 45 additions and 28 deletions

View file

@ -6,7 +6,6 @@ package git
import ( import (
"context" "context"
"fmt" "fmt"
"path"
"strings" "strings"
"time" "time"
@ -17,6 +16,7 @@ import (
"code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"
) )
// LFSLock represents a git lfs lock of repository. // 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. // BeforeInsert is invoked from XORM before inserting an object of this type.
func (l *LFSLock) BeforeInsert() { func (l *LFSLock) BeforeInsert() {
l.Path = cleanPath(l.Path) l.Path = util.CleanPath(l.Path)
}
func cleanPath(p string) string {
return path.Clean("/" + p)[1:]
} }
// CreateLFSLock creates a new lock. // CreateLFSLock creates a new lock.
@ -53,7 +49,7 @@ func CreateLFSLock(ctx context.Context, repo *repo_model.Repository, lock *LFSLo
return nil, err return nil, err
} }
lock.Path = cleanPath(lock.Path) lock.Path = util.CleanPath(lock.Path)
lock.RepoID = repo.ID lock.RepoID = repo.ID
l, err := GetLFSLock(dbCtx, repo, lock.Path) 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. // GetLFSLock returns release by given path.
func GetLFSLock(ctx context.Context, repo *repo_model.Repository, path string) (*LFSLock, error) { 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} rel := &LFSLock{RepoID: repo.ID}
has, err := db.GetEngine(ctx).Where("lower(path) = ?", strings.ToLower(path)).Get(rel) has, err := db.GetEngine(ctx).Where("lower(path) = ?", strings.ToLower(path)).Get(rel)
if err != nil { if err != nil {

View file

@ -16,27 +16,27 @@ import (
// Locale reads the content of a specific locale from static/bindata or custom path. // Locale reads the content of a specific locale from static/bindata or custom path.
func Locale(name string) ([]byte, error) { 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. // Readme reads the content of a specific readme from static/bindata or custom path.
func Readme(name string) ([]byte, error) { 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. // Gitignore reads the content of a gitignore locale from static/bindata or custom path.
func Gitignore(name string) ([]byte, error) { 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. // License reads the content of a specific license from static/bindata or custom path.
func License(name string) ([]byte, error) { 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. // Labels reads the content of a specific labels from static/bindata or custom path.
func Labels(name string) ([]byte, error) { 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 // WalkLocales reads the content of a specific locale

View file

@ -6,7 +6,6 @@ package public
import ( import (
"net/http" "net/http"
"os" "os"
"path"
"path/filepath" "path/filepath"
"strings" "strings"
@ -14,6 +13,7 @@ import (
"code.gitea.io/gitea/modules/httpcache" "code.gitea.io/gitea/modules/httpcache"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"
) )
// Options represents the available options to configure the handler. // 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 { 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 .. // 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 err != nil {
if os.IsNotExist(err) { if os.IsNotExist(err) {
return false return false

View file

@ -8,7 +8,6 @@ import (
"io" "io"
"net/url" "net/url"
"os" "os"
"path"
"path/filepath" "path/filepath"
"strings" "strings"
@ -59,7 +58,7 @@ func NewLocalStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error
} }
func (l *LocalStorage) buildLocalPath(p string) string { 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 // Open a file

View file

@ -15,6 +15,7 @@ import (
"time" "time"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/util"
"github.com/minio/minio-go/v7" "github.com/minio/minio-go/v7"
"github.com/minio/minio-go/v7/pkg/credentials" "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 { 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 // Open open a file

View file

@ -14,6 +14,14 @@ import (
"strings" "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 // EnsureAbsolutePath ensure that a path is absolute, making it
// relative to absoluteBase if necessary // relative to absoluteBase if necessary
func EnsureAbsolutePath(path, absoluteBase string) string { func EnsureAbsolutePath(path, absoluteBase string) string {

View file

@ -136,3 +136,15 @@ func TestMisc_IsReadmeFileName(t *testing.T) {
assert.Equal(t, testCase.idx, idx) 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))
}
}

View file

@ -19,6 +19,7 @@ import (
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/storage"
"code.gitea.io/gitea/modules/templates" "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/middleware"
"code.gitea.io/gitea/modules/web/routing" "code.gitea.io/gitea/modules/web/routing"
"code.gitea.io/gitea/services/auth" "code.gitea.io/gitea/services/auth"
@ -44,7 +45,7 @@ func storageHandler(storageSetting setting.Storage, prefix string, objStore stor
routing.UpdateFuncInfo(req.Context(), funcInfo) routing.UpdateFuncInfo(req.Context(), funcInfo)
rPath := strings.TrimPrefix(req.URL.Path, "/"+prefix+"/") 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)) u, err := objStore.URL(rPath, path.Base(rPath))
if err != nil { if err != nil {
@ -80,7 +81,7 @@ func storageHandler(storageSetting setting.Storage, prefix string, objStore stor
routing.UpdateFuncInfo(req.Context(), funcInfo) routing.UpdateFuncInfo(req.Context(), funcInfo)
rPath := strings.TrimPrefix(req.URL.Path, "/"+prefix+"/") rPath := strings.TrimPrefix(req.URL.Path, "/"+prefix+"/")
rPath = path.Clean("/" + strings.ReplaceAll(rPath, "\\", "/"))[1:] rPath = util.CleanPath(strings.ReplaceAll(rPath, "\\", "/"))
if rPath == "" { if rPath == "" {
http.Error(w, "file not found", http.StatusNotFound) http.Error(w, "file not found", http.StatusNotFound)
return return

View file

@ -726,7 +726,7 @@ func UploadFilePost(ctx *context.Context) {
func cleanUploadFileName(name string) string { func cleanUploadFileName(name string) string {
// Rebase the filename // 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. // Git disallows any filenames to have a .git directory in them.
for _, part := range strings.Split(name, "/") { for _, part := range strings.Split(name, "/") {
if strings.ToLower(part) == ".git" { if strings.ToLower(part) == ".git" {

View file

@ -207,7 +207,7 @@ func LFSLockFile(ctx *context.Context) {
ctx.Redirect(ctx.Repo.RepoLink + "/settings/lfs/locks") ctx.Redirect(ctx.Repo.RepoLink + "/settings/lfs/locks")
return return
} }
lockPath = path.Clean("/" + lockPath)[1:] lockPath = util.CleanPath(lockPath)
if len(lockPath) == 0 { if len(lockPath) == 0 {
ctx.Flash.Error(ctx.Tr("repo.settings.lfs_invalid_locking_path", originalPath)) ctx.Flash.Error(ctx.Tr("repo.settings.lfs_invalid_locking_path", originalPath))
ctx.Redirect(ctx.Repo.RepoLink + "/settings/lfs/locks") ctx.Redirect(ctx.Repo.RepoLink + "/settings/lfs/locks")

View file

@ -9,7 +9,6 @@ import (
"fmt" "fmt"
"io" "io"
"os" "os"
"path"
"path/filepath" "path/filepath"
"strconv" "strconv"
"strings" "strings"
@ -30,6 +29,7 @@ import (
"code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/uri" "code.gitea.io/gitea/modules/uri"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/services/pull" "code.gitea.io/gitea/services/pull"
"github.com/google/uuid" "github.com/google/uuid"
@ -866,7 +866,7 @@ func (g *GiteaLocalUploader) CreateReviews(reviews ...*base.Review) error {
} }
// SECURITY: The TreePath must be cleaned! // SECURITY: The TreePath must be cleaned!
comment.TreePath = path.Clean("/" + comment.TreePath)[1:] comment.TreePath = util.CleanPath(comment.TreePath)
var patch string var patch string
reader, writer := io.Pipe() reader, writer := io.Pipe()

View file

@ -8,13 +8,13 @@ import (
"errors" "errors"
"io" "io"
"os" "os"
"path"
"path/filepath" "path/filepath"
"strings" "strings"
packages_model "code.gitea.io/gitea/models/packages" packages_model "code.gitea.io/gitea/models/packages"
packages_module "code.gitea.io/gitea/modules/packages" packages_module "code.gitea.io/gitea/modules/packages"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"
) )
var ( var (
@ -33,7 +33,7 @@ type BlobUploader struct {
} }
func buildFilePath(id string) string { 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 // NewBlobUploader creates a new blob uploader for the given id

View file

@ -7,7 +7,6 @@ import (
"context" "context"
"fmt" "fmt"
"net/url" "net/url"
"path"
"strings" "strings"
"time" "time"
@ -15,6 +14,7 @@ import (
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git"
api "code.gitea.io/gitea/modules/structs" api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/util"
) )
// GetFileResponseFromCommit Constructs a FileResponse from a Commit object // 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 // CleanUploadFileName Trims a filename and returns empty string if it is a .git directory
func CleanUploadFileName(name string) string { func CleanUploadFileName(name string) string {
// Rebase the filename // 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. // Git disallows any filenames to have a .git directory in them.
for _, part := range strings.Split(name, "/") { for _, part := range strings.Split(name, "/") {
if strings.ToLower(part) == ".git" { if strings.ToLower(part) == ".git" {