From a8d1bf3f9349c138383b65079b7b8ad97fff78f4 Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Tue, 7 Jan 2025 14:30:47 +0300 Subject: [PATCH] fix: prevent path traversal attacks (#631) This commit fixes a path traversal vulnerability in the repository management code. The `SanitizeRepo` function now correctly returns a sanitized version of the given repository name. It uses an absolute path along with `path.Clean` to ensure that the path is cleaned before being used. --- pkg/backend/repo.go | 37 ++++++++++++++++++------------------- pkg/ssh/cmd/cmd.go | 2 +- pkg/utils/utils.go | 5 ++++- 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/pkg/backend/repo.go b/pkg/backend/repo.go index bff5658e4..a4a087fe5 100644 --- a/pkg/backend/repo.go +++ b/pkg/backend/repo.go @@ -10,6 +10,7 @@ import ( "path" "path/filepath" "strconv" + "strings" "time" "github.com/charmbracelet/soft-serve/git" @@ -24,10 +25,6 @@ import ( "github.com/charmbracelet/soft-serve/pkg/webhook" ) -func (d *Backend) reposPath() string { - return filepath.Join(d.cfg.DataPath, "repos") -} - // CreateRepository creates a new repository. // // It implements backend.Backend. @@ -37,8 +34,7 @@ func (d *Backend) CreateRepository(ctx context.Context, name string, user proto. return nil, err } - repo := name + ".git" - rp := filepath.Join(d.reposPath(), repo) + rp := filepath.Join(d.repoPath(name)) var userID int64 if user != nil { @@ -78,7 +74,7 @@ func (d *Backend) CreateRepository(ctx context.Context, name string, user proto. } } - return hooks.GenerateHooks(ctx, d.cfg, repo) + return hooks.GenerateHooks(ctx, d.cfg, name) }); err != nil { d.logger.Debug("failed to create repository in database", "err", err) err = db.WrapError(err) @@ -100,8 +96,7 @@ func (d *Backend) ImportRepository(_ context.Context, name string, user proto.Us return nil, err } - repo := name + ".git" - rp := filepath.Join(d.reposPath(), repo) + rp := filepath.Join(d.repoPath(name)) tid := "import:" + name if d.manager.Exists(tid) { @@ -217,8 +212,7 @@ func (d *Backend) ImportRepository(_ context.Context, name string, user proto.Us // It implements backend.Backend. func (d *Backend) DeleteRepository(ctx context.Context, name string) error { name = utils.SanitizeRepo(name) - repo := name + ".git" - rp := filepath.Join(d.reposPath(), repo) + rp := filepath.Join(d.repoPath(name)) user := proto.UserFromContext(ctx) r, err := d.Repository(ctx, name) @@ -330,10 +324,8 @@ func (d *Backend) RenameRepository(ctx context.Context, oldName string, newName return nil } - oldRepo := oldName + ".git" - newRepo := newName + ".git" - op := filepath.Join(d.reposPath(), oldRepo) - np := filepath.Join(d.reposPath(), newRepo) + op := filepath.Join(d.repoPath(oldName)) + np := filepath.Join(d.repoPath(newName)) if _, err := os.Stat(op); err != nil { return proto.ErrRepoNotFound } @@ -389,7 +381,7 @@ func (d *Backend) Repositories(ctx context.Context) ([]proto.Repository, error) for _, m := range ms { r := &repo{ name: m.Name, - path: filepath.Join(d.reposPath(), m.Name+".git"), + path: filepath.Join(d.repoPath(m.Name)), repo: m, } @@ -418,7 +410,7 @@ func (d *Backend) Repository(ctx context.Context, name string) (proto.Repository return r, nil } - rp := filepath.Join(d.reposPath(), name+".git") + rp := filepath.Join(d.repoPath(name)) if _, err := os.Stat(rp); err != nil { if !errors.Is(err, fs.ErrNotExist) { d.logger.Errorf("failed to stat repository path: %v", err) @@ -552,7 +544,7 @@ func (d *Backend) SetHidden(ctx context.Context, name string, hidden bool) error // It implements backend.Backend. func (d *Backend) SetDescription(ctx context.Context, name string, desc string) error { name = utils.SanitizeRepo(name) - rp := filepath.Join(d.reposPath(), name+".git") + rp := filepath.Join(d.repoPath(name)) // Delete cache d.cache.Delete(name) @@ -572,7 +564,7 @@ func (d *Backend) SetDescription(ctx context.Context, name string, desc string) // It implements backend.Backend. func (d *Backend) SetPrivate(ctx context.Context, name string, private bool) error { name = utils.SanitizeRepo(name) - rp := filepath.Join(d.reposPath(), name+".git") + rp := filepath.Join(d.repoPath(name)) // Delete cache d.cache.Delete(name) @@ -636,6 +628,13 @@ func (d *Backend) SetProjectName(ctx context.Context, repo string, name string) ) } +// repoPath returns the path to a repository. +func (d *Backend) repoPath(name string) string { + name = utils.SanitizeRepo(name) + rn := strings.ReplaceAll(name, "/", string(os.PathSeparator)) + return filepath.Join(filepath.Join(d.cfg.DataPath, "repos"), rn+".git") +} + var _ proto.Repository = (*repo)(nil) // repo is a Git repository with metadata stored in a SQLite database. diff --git a/pkg/ssh/cmd/cmd.go b/pkg/ssh/cmd/cmd.go index 18624eb33..61645966d 100644 --- a/pkg/ssh/cmd/cmd.go +++ b/pkg/ssh/cmd/cmd.go @@ -172,7 +172,7 @@ func checkIfAdmin(cmd *cobra.Command, args []string) error { func checkIfCollab(cmd *cobra.Command, args []string) error { var repo string if len(args) > 0 { - repo = args[0] + repo = utils.SanitizeRepo(args[0]) } ctx := cmd.Context() diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index a98cb139c..559e8e1b7 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -9,12 +9,15 @@ import ( // SanitizeRepo returns a sanitized version of the given repository name. func SanitizeRepo(repo string) string { + // We need to use an absolute path for the path to be cleaned correctly. repo = strings.TrimPrefix(repo, "/") + repo = "/" + repo + // We're using path instead of filepath here because this is not OS dependent // looking at you Windows repo = path.Clean(repo) repo = strings.TrimSuffix(repo, ".git") - return repo + return repo[1:] } // ValidateUsername returns an error if any of the given usernames are invalid.