Nemcio/Gogs
1
0
Fork 0
mirror of https://github.com/gogs/gogs.git synced 2026-01-26 17:19:17 +01:00
Code Issues Packages Projects Releases Wiki Activity

repository: reject any updates that has symlink in path hierarchy (#8082)

Browse Source 
Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
ᴊᴏᴇ ᴄʜᴇɴ
2026-01-08 14:22:04 -05:00
committed by Joe Chen
parent 33990972fa
commit c3eca1fca3
2 changed files with 59 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

105
internal/db/repo_editor.go
View File

@@ -107,6 +107,19 @@ func (repo *Repository) CheckoutNewBranch(oldBranch, newBranch string) error {
return nil
}
// hasSymlinkInPath returns true if there is any symlink in path hierarchy using
// the given base and relative path.
func hasSymlinkInPath(base, relPath string) bool {
parts := strings.Split(filepath.ToSlash(relPath), "/")
for i := range parts {
filePath := path.Join(append([]string{base}, parts[:i+1]...)...)
if osutil.IsSymlink(filePath) {
return true
}
}
return false
}
type UpdateRepoFileOptions struct {
OldBranch string
NewBranch string
@@ -118,7 +131,7 @@ type UpdateRepoFileOptions struct {
}
// UpdateRepoFile adds or updates a file in repository.
func (repo *Repository) UpdateRepoFile(doer *User, opts UpdateRepoFileOptions) (err error) {
func (repo *Repository) UpdateRepoFile(doer *User, opts UpdateRepoFileOptions) error {
// 🚨 SECURITY: Prevent uploading files into the ".git" directory.
if isRepositoryGitPath(opts.NewTreeName) {
return errors.Errorf("bad tree path %q", opts.NewTreeName)
@@ -127,15 +140,21 @@ func (repo *Repository) UpdateRepoFile(doer *User, opts UpdateRepoFileOptions) (
repoWorkingPool.CheckIn(com.ToStr(repo.ID))
defer repoWorkingPool.CheckOut(com.ToStr(repo.ID))
if err = repo.DiscardLocalRepoBranchChanges(opts.OldBranch); err != nil {
if err := repo.DiscardLocalRepoBranchChanges(opts.OldBranch); err != nil {
return fmt.Errorf("discard local repo branch[%s] changes: %v", opts.OldBranch, err)
} else if err = repo.UpdateLocalCopyBranch(opts.OldBranch); err != nil {
return fmt.Errorf("update local copy branch[%s]: %v", opts.OldBranch, err)
}
repoPath := repo.RepoPath()
localPath := repo.LocalCopyPath()
// 🚨 SECURITY: Prevent touching files in surprising places, reject operations
// that involve symlinks.
if hasSymlinkInPath(localPath, opts.OldTreeName) || hasSymlinkInPath(localPath, opts.NewTreeName) {
return errors.New("cannot update file with symbolic link in path")
}
repoPath := repo.RepoPath()
if opts.OldBranch != opts.NewBranch {
// Directly return error if new branch already exists in the server
if git.RepoHasBranch(repoPath, opts.NewBranch) {
@@ -144,7 +163,7 @@ func (repo *Repository) UpdateRepoFile(doer *User, opts UpdateRepoFileOptions) (
// Otherwise, delete branch from local copy in case out of sync
if git.RepoHasBranch(localPath, opts.NewBranch) {
if err = git.DeleteBranch(localPath, opts.NewBranch, git.DeleteBranchOptions{
if err := git.DeleteBranch(localPath, opts.NewBranch, git.DeleteBranchOptions{
Force: true,
}); err != nil {
return fmt.Errorf("delete branch %q: %v", opts.NewBranch, err)
@@ -157,46 +176,31 @@ func (repo *Repository) UpdateRepoFile(doer *User, opts UpdateRepoFileOptions) (
}
oldFilePath := path.Join(localPath, opts.OldTreeName)
filePath := path.Join(localPath, opts.NewTreeName)
if err = os.MkdirAll(path.Dir(filePath), os.ModePerm); err != nil {
return err
newFilePath := path.Join(localPath, opts.NewTreeName)
// Prompt the user if the meant-to-be new file already exists.
if osutil.IsExist(newFilePath) && opts.IsNewFile {
return ErrRepoFileAlreadyExist{newFilePath}
}
// If it's meant to be a new file, make sure it doesn't exist.
if opts.IsNewFile {
// 🚨 SECURITY: Prevent updating files in surprising place, check if the file is
// a symlink.
if osutil.IsSymlink(filePath) {
return fmt.Errorf("cannot update symbolic link: %s", opts.NewTreeName)
}
if osutil.IsExist(filePath) {
return ErrRepoFileAlreadyExist{filePath}
}
if err := os.MkdirAll(path.Dir(newFilePath), os.ModePerm); err != nil {
return errors.Wrapf(err, "create parent directories of %q", newFilePath)
}
// Ignore move step if it's a new file under a directory.
// Otherwise, move the file when name changed.
if osutil.IsFile(oldFilePath) && opts.OldTreeName != opts.NewTreeName {
// 🚨 SECURITY: Prevent updating files in surprising place, check if the file is
// a symlink.
if osutil.IsSymlink(oldFilePath) {
return fmt.Errorf("cannot move symbolic link: %s", opts.OldTreeName)
}
if err = git.Move(localPath, opts.OldTreeName, opts.NewTreeName); err != nil {
return fmt.Errorf("git mv %q %q: %v", opts.OldTreeName, opts.NewTreeName, err)
if err := git.Move(localPath, opts.OldTreeName, opts.NewTreeName); err != nil {
return errors.Wrapf(err, "git mv %q %q", opts.OldTreeName, opts.NewTreeName)
}
}
if err = os.WriteFile(filePath, []byte(opts.Content), 0600); err != nil {
if err := os.WriteFile(newFilePath, []byte(opts.Content), 0o600); err != nil {
return fmt.Errorf("write file: %v", err)
}
if err = git.Add(localPath, git.AddOptions{All: true}); err != nil {
if err := git.Add(localPath, git.AddOptions{All: true}); err != nil {
return fmt.Errorf("git add --all: %v", err)
}
err = git.CreateCommit(
err := git.CreateCommit(
localPath,
&git.Signature{
Name: doer.DisplayName(),
@@ -230,7 +234,7 @@ func (repo *Repository) UpdateRepoFile(doer *User, opts UpdateRepoFileOptions) (
}
// GetDiffPreview produces and returns diff result of a file which is not yet committed.
func (repo *Repository) GetDiffPreview(branch, treePath, content string) (diff *gitutil.Diff, err error) {
func (repo *Repository) GetDiffPreview(branch, treePath, content string) (*gitutil.Diff, error) {
// 🚨 SECURITY: Prevent uploading files into the ".git" directory.
if isRepositoryGitPath(treePath) {
return nil, errors.Errorf("bad tree path %q", treePath)
@@ -239,7 +243,7 @@ func (repo *Repository) GetDiffPreview(branch, treePath, content string) (diff *
repoWorkingPool.CheckIn(com.ToStr(repo.ID))
defer repoWorkingPool.CheckOut(com.ToStr(repo.ID))
if err = repo.DiscardLocalRepoBranchChanges(branch); err != nil {
if err := repo.DiscardLocalRepoBranchChanges(branch); err != nil {
return nil, fmt.Errorf("discard local repo branch[%s] changes: %v", branch, err)
} else if err = repo.UpdateLocalCopyBranch(branch); err != nil {
return nil, fmt.Errorf("update local copy branch[%s]: %v", branch, err)
@@ -248,14 +252,15 @@ func (repo *Repository) GetDiffPreview(branch, treePath, content string) (diff *
localPath := repo.LocalCopyPath()
filePath := path.Join(localPath, treePath)
// 🚨 SECURITY: Prevent updating files in surprising place, check if the target is
// a symlink.
if osutil.IsSymlink(filePath) {
return nil, fmt.Errorf("cannot get diff preview for symbolic link: %s", treePath)
// 🚨 SECURITY: Prevent touching files in surprising places, reject operations
// that involve symlinks.
if hasSymlinkInPath(localPath, treePath) {
return nil, errors.New("cannot update file with symbolic link in path")
}
if err = os.MkdirAll(filepath.Dir(filePath), os.ModePerm); err != nil {
return nil, err
} else if err = os.WriteFile(filePath, []byte(content), 0600); err != nil {
if err := os.MkdirAll(path.Dir(filePath), os.ModePerm); err != nil {
return nil, errors.Wrap(err, "create parent directories")
} else if err = os.WriteFile(filePath, []byte(content), 0o600); err != nil {
return nil, fmt.Errorf("write file: %v", err)
}
@@ -276,15 +281,13 @@ func (repo *Repository) GetDiffPreview(branch, treePath, content string) (diff *
pid := process.Add(fmt.Sprintf("GetDiffPreview [repo_path: %s]", repo.RepoPath()), cmd)
defer process.Remove(pid)
diff, err = gitutil.ParseDiff(stdout, conf.Git.MaxDiffFiles, conf.Git.MaxDiffLines, conf.Git.MaxDiffLineChars)
diff, err := gitutil.ParseDiff(stdout, conf.Git.MaxDiffFiles, conf.Git.MaxDiffLines, conf.Git.MaxDiffLineChars)
if err != nil {
return nil, fmt.Errorf("parse diff: %v", err)
}
if err = cmd.Wait(); err != nil {
return nil, fmt.Errorf("wait: %v", err)
}
return diff, nil
}
@@ -319,21 +322,21 @@ func (repo *Repository) DeleteRepoFile(doer *User, opts DeleteRepoFileOptions) (
return fmt.Errorf("update local copy branch[%s]: %v", opts.OldBranch, err)
}
localPath := repo.LocalCopyPath()
// 🚨 SECURITY: Prevent touching files in surprising places, reject operations
// that involve symlinks.
if hasSymlinkInPath(localPath, opts.TreePath) {
return errors.New("cannot update file with symbolic link in path")
}
if opts.OldBranch != opts.NewBranch {
if err := repo.CheckoutNewBranch(opts.OldBranch, opts.NewBranch); err != nil {
return fmt.Errorf("checkout new branch[%s] from old branch[%s]: %v", opts.NewBranch, opts.OldBranch, err)
}
}
localPath := repo.LocalCopyPath()
filePath := path.Join(localPath, opts.TreePath)
// 🚨 SECURITY: Prevent updating files in surprising place, check if the file is
// a symlink.
if osutil.IsSymlink(filePath) {
return fmt.Errorf("cannot delete symbolic link: %s", opts.TreePath)
}
if err = os.Remove(filePath); err != nil {
return fmt.Errorf("remove file %q: %v", opts.TreePath, err)
}

14
internal/osutil/osutil.go
View File

@@ -9,7 +9,8 @@ import (
"os/user"
)
// IsFile returns true if given path exists as a file (i.e. not a directory).
// IsFile returns true if given path exists as a file (i.e. not a directory)
// following any symlinks.
func IsFile(path string) bool {
f, e := os.Stat(path)
if e != nil {
@@ -18,8 +19,8 @@ func IsFile(path string) bool {
return !f.IsDir()
}
// IsDir returns true if given path is a directory, and returns false when it's
// a file or does not exist.
// IsDir returns true if given path is a directory following any symlinks, and
// returns false when it's a file or does not exist.
func IsDir(dir string) bool {
f, e := os.Stat(dir)
if e != nil {
@@ -28,7 +29,7 @@ func IsDir(dir string) bool {
return f.IsDir()
}
// IsExist returns true if a file or directory exists.
// IsExist returns true if a file or directory exists following any symlinks.
func IsExist(path string) bool {
_, err := os.Stat(path)
return err == nil || os.IsExist(err)
@@ -36,15 +37,10 @@ func IsExist(path string) bool {
// IsSymlink returns true if given path is a symbolic link.
func IsSymlink(path string) bool {
if !IsExist(path) {
return false
}
fileInfo, err := os.Lstat(path)
if err != nil {
return false
}
return fileInfo.Mode()&os.ModeSymlink != 0
}