a8d1bf3f9349c138383b65079b7b8ad97fff78f4

Message

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.

Diff

This diff is truncated to protect this page.

  1diff --git a/pkg/backend/repo.go b/pkg/backend/repo.go
  2index bff5658e44718688e9cbdc25fd1314dfdcd01b58..a4a087fe5a78b3ab4ed0c3a1a38b0505cb432284 100644
  3--- a/pkg/backend/repo.go
  4+++ b/pkg/backend/repo.go
  5@@ -10,6 +10,7 @@ import (
  6 	"path"
  7 	"path/filepath"
  8 	"strconv"
  9+	"strings"
 10 	"time"
 11 
 12 	"github.com/charmbracelet/soft-serve/git"
 13@@ -24,10 +25,6 @@ import (
 14 	"github.com/charmbracelet/soft-serve/pkg/webhook"
 15 )
 16 
 17-func (d *Backend) reposPath() string {
 18-	return filepath.Join(d.cfg.DataPath, "repos")
 19-}
 20-
 21 // CreateRepository creates a new repository.
 22 //
 23 // It implements backend.Backend.
 24@@ -37,8 +34,7 @@ func (d *Backend) CreateRepository(ctx context.Context, name string, user proto.
 25 		return nil, err
 26 	}
 27 
 28-	repo := name + ".git"
 29-	rp := filepath.Join(d.reposPath(), repo)
 30+	rp := filepath.Join(d.repoPath(name))
 31 
 32 	var userID int64
 33 	if user != nil {
 34@@ -78,7 +74,7 @@ func (d *Backend) CreateRepository(ctx context.Context, name string, user proto.
 35 			}
 36 		}
 37 
 38-		return hooks.GenerateHooks(ctx, d.cfg, repo)
 39+		return hooks.GenerateHooks(ctx, d.cfg, name)
 40 	}); err != nil {
 41 		d.logger.Debug("failed to create repository in database", "err", err)
 42 		err = db.WrapError(err)
 43@@ -100,8 +96,7 @@ func (d *Backend) ImportRepository(_ context.Context, name string, user proto.Us
 44 		return nil, err
 45 	}
 46 
 47-	repo := name + ".git"
 48-	rp := filepath.Join(d.reposPath(), repo)
 49+	rp := filepath.Join(d.repoPath(name))
 50 
 51 	tid := "import:" + name
 52 	if d.manager.Exists(tid) {
 53@@ -217,8 +212,7 @@ func (d *Backend) ImportRepository(_ context.Context, name string, user proto.Us
 54 // It implements backend.Backend.
 55 func (d *Backend) DeleteRepository(ctx context.Context, name string) error {
 56 	name = utils.SanitizeRepo(name)
 57-	repo := name + ".git"
 58-	rp := filepath.Join(d.reposPath(), repo)
 59+	rp := filepath.Join(d.repoPath(name))
 60 
 61 	user := proto.UserFromContext(ctx)
 62 	r, err := d.Repository(ctx, name)
 63@@ -330,10 +324,8 @@ func (d *Backend) RenameRepository(ctx context.Context, oldName string, newName
 64 		return nil
 65 	}
 66 
 67-	oldRepo := oldName + ".git"
 68-	newRepo := newName + ".git"
 69-	op := filepath.Join(d.reposPath(), oldRepo)
 70-	np := filepath.Join(d.reposPath(), newRepo)
 71+	op := filepath.Join(d.repoPath(oldName))
 72+	np := filepath.Join(d.repoPath(newName))
 73 	if _, err := os.Stat(op); err != nil {
 74 		return proto.ErrRepoNotFound
 75 	}
 76@@ -389,7 +381,7 @@ func (d *Backend) Repositories(ctx context.Context) ([]proto.Repository, error)
 77 		for _, m := range ms {
 78 			r := &repo{
 79 				name: m.Name,
 80-				path: filepath.Join(d.reposPath(), m.Name+".git"),
 81+				path: filepath.Join(d.repoPath(m.Name)),
 82 				repo: m,
 83 			}
 84 
 85@@ -418,7 +410,7 @@ func (d *Backend) Repository(ctx context.Context, name string) (proto.Repository
 86 		return r, nil
 87 	}
 88 
 89-	rp := filepath.Join(d.reposPath(), name+".git")
 90+	rp := filepath.Join(d.repoPath(name))
 91 	if _, err := os.Stat(rp); err != nil {
 92 		if !errors.Is(err, fs.ErrNotExist) {
 93 			d.logger.Errorf("failed to stat repository path: %v", err)
 94@@ -552,7 +544,7 @@ func (d *Backend) SetHidden(ctx context.Context, name string, hidden bool) error
 95 // It implements backend.Backend.
 96 func (d *Backend) SetDescription(ctx context.Context, name string, desc string) error {
 97 	name = utils.SanitizeRepo(name)
 98-	rp := filepath.Join(d.reposPath(), name+".git")
 99+	rp := filepath.Join(d.repoPath(name))
100 
101 	// Delete cache
102 	d.cache.Delete(name)
103@@ -572,7 +564,7 @@ func (d *Backend) SetDescription(ctx context.Context, name string, desc string)
104 // It implements backend.Backend.
105diff --git a/pkg/ssh/cmd/cmd.go b/pkg/ssh/cmd/cmd.go
106index 18624eb333adb56ba7992c154c79d15fad20f43b..61645966d252f9d82266164c756b9d8e1a587125 100644
107--- a/pkg/ssh/cmd/cmd.go
108+++ b/pkg/ssh/cmd/cmd.go
109@@ -172,7 +172,7 @@ func checkIfAdmin(cmd *cobra.Command, args []string) error {
110 func checkIfCollab(cmd *cobra.Command, args []string) error {
111 	var repo string
112 	if len(args) > 0 {
113-		repo = args[0]
114+		repo = utils.SanitizeRepo(args[0])
115 	}
116 
117 	ctx := cmd.Context()
118diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go
119index a98cb139c51e853c45a6c9630e277f9bd313403f..559e8e1b70324bdcfc049d3a4140dd1bf0e7f6aa 100644
120--- a/pkg/utils/utils.go
121+++ b/pkg/utils/utils.go
122@@ -9,12 +9,15 @@ import (
123 
124 // SanitizeRepo returns a sanitized version of the given repository name.
125 func SanitizeRepo(repo string) string {
126+	// We need to use an absolute path for the path to be cleaned correctly.
127 	repo = strings.TrimPrefix(repo, "/")
128+	repo = "/" + repo
129+
130 	// We're using path instead of filepath here because this is not OS dependent
131 	// looking at you Windows
132 	repo = path.Clean(repo)
133 	repo = strings.TrimSuffix(repo, ".git")
134-	return repo
135+	return repo[1:]
136 }
137 
138 // ValidateUsername returns an error if any of the given usernames are invalid.