9c1c342df8a62ebc1020d303b0d93191dbf28cd4

Message

refactor(git): sanitize repo name

Diff

This diff is truncated to protect this page.

  1diff --git a/server/daemon.go b/server/daemon.go
  2index 0055ff9c6ce05726dbdb39d3e51eaeabc5935be0..34600792da6c289c3cb860d30849a07bf73093b7 100644
  3--- a/server/daemon.go
  4+++ b/server/daemon.go
  5@@ -5,10 +5,8 @@ import (
  6 	"context"
  7 	"errors"
  8 	"io"
  9-	"log"
 10 	"net"
 11 	"path/filepath"
 12-	"strings"
 13 	"sync"
 14 	"time"
 15 
 16@@ -97,7 +95,7 @@ func (d *GitDaemon) Start() error {
 17 			case <-d.finished:
 18 				return ErrServerClosed
 19 			default:
 20-				log.Printf("git: error accepting connection: %v", err)
 21+				logger.Debugf("git: error accepting connection: %v", err)
 22 			}
 23 			if ne, ok := err.(net.Error); ok && ne.Temporary() {
 24 				if tempDelay == 0 {
 25@@ -116,7 +114,7 @@ func (d *GitDaemon) Start() error {
 26 
 27 		// Close connection if there are too many open connections.
 28 		if d.conns.Size()+1 >= d.cfg.Git.MaxConnections {
 29-			log.Printf("git: max connections reached, closing %s", conn.RemoteAddr())
 30+			logger.Debugf("git: max connections reached, closing %s", conn.RemoteAddr())
 31 			fatal(conn, ErrMaxConnections)
 32 			continue
 33 		}
 34@@ -132,7 +130,7 @@ func (d *GitDaemon) Start() error {
 35 func fatal(c net.Conn, err error) {
 36 	WritePktline(c, err)
 37 	if err := c.Close(); err != nil {
 38-		log.Printf("git: error closing connection: %v", err)
 39+		logger.Debugf("git: error closing connection: %v", err)
 40 	}
 41 }
 42 
 43@@ -162,7 +160,7 @@ func (d *GitDaemon) handleClient(conn net.Conn) {
 44 				if nerr, ok := err.(net.Error); ok && nerr.Timeout() {
 45 					fatal(c, ErrTimeout)
 46 				} else {
 47-					log.Printf("git: error scanning pktline: %v", err)
 48+					logger.Debugf("git: error scanning pktline: %v", err)
 49 					fatal(c, ErrSystemMalfunction)
 50 				}
 51 			}
 52@@ -174,7 +172,7 @@ func (d *GitDaemon) handleClient(conn net.Conn) {
 53 	select {
 54 	case <-ctx.Done():
 55 		if err := ctx.Err(); err != nil {
 56-			log.Printf("git: connection context error: %v", err)
 57+			logger.Debugf("git: connection context error: %v", err)
 58 		}
 59 		return
 60 	case <-readc:
 61@@ -186,7 +184,6 @@ func (d *GitDaemon) handleClient(conn net.Conn) {
 62 		}
 63 
 64 		var gitPack func(io.Reader, io.Writer, io.Writer, string) error
 65-		var repo string
 66 		cmd := string(split[0])
 67 		switch cmd {
 68 		case UploadPackBin:
 69@@ -204,21 +201,26 @@ func (d *GitDaemon) handleClient(conn net.Conn) {
 70 			return
 71 		}
 72 
 73-		repo = filepath.Clean(string(opts[0]))
 74-		log.Printf("git: connect %s %s %s", c.RemoteAddr(), cmd, repo)
 75-		defer log.Printf("git: disconnect %s %s %s", c.RemoteAddr(), cmd, repo)
 76-		repo = strings.TrimPrefix(repo, "/")
 77-		auth := d.cfg.Access.AccessLevel(strings.TrimSuffix(repo, ".git"), nil)
 78+		name := sanitizeRepoName(string(opts[0]))
 79+		logger.Debugf("git: connect %s %s %s", c.RemoteAddr(), cmd, name)
 80+		defer logger.Debugf("git: disconnect %s %s %s", c.RemoteAddr(), cmd, name)
 81+		// git bare repositories should end in ".git"
 82+		// https://git-scm.com/docs/gitrepository-layout
 83+		repo := name + ".git"
 84+		// FIXME: determine repositories path
 85+		reposDir := filepath.Join(d.cfg.DataPath, "repos")
 86+		if err := ensureWithin(reposDir, repo); err != nil {
 87+			fatal(c, err)
 88+			return
 89+		}
 90+
 91+		auth := d.cfg.Access.AccessLevel(name, nil)
 92 		if auth < backend.ReadOnlyAccess {
 93 			fatal(c, ErrNotAuthed)
 94 			return
 95 		}
 96-		// git bare repositories should end in ".git"
 97-		// https://git-scm.com/docs/gitrepository-layout
 98-		repo = strings.TrimSuffix(repo, ".git") + ".git"
 99-		// FIXME: determine repositories path
100-		repoDir := filepath.Join(d.cfg.DataPath, "repos", repo)
101-		if err := gitPack(c, c, c, repoDir); err != nil {
102+
103+		if err := gitPack(c, c, c, filepath.Join(reposDir, repo)); err != nil {
104 			fatal(c, err)
105diff --git a/server/git.go b/server/git.go
106index 812dddfc46c37f6952bcd2bfebaf7286a16d129e..1748f1ec4d7a1fcc6579a1b0ab7a1b6f4b726d2e 100644
107--- a/server/git.go
108+++ b/server/git.go
109@@ -4,10 +4,11 @@ import (
110 	"errors"
111 	"fmt"
112 	"io"
113-	"log"
114 	"os"
115 	"path/filepath"
116+	"strings"
117 
118+	"github.com/charmbracelet/log"
119 	"github.com/charmbracelet/soft-serve/git"
120 	"github.com/go-git/go-git/v5/plumbing/format/pktline"
121 )
122@@ -90,13 +91,36 @@ func WritePktline(w io.Writer, v ...interface{}) {
123 	msg := fmt.Sprintln(v...)
124 	pkt := pktline.NewEncoder(w)
125 	if err := pkt.EncodeString(msg); err != nil {
126-		log.Printf("git: error writing pkt-line message: %s", err)
127+		log.Debugf("git: error writing pkt-line message: %s", err)
128 	}
129 	if err := pkt.Flush(); err != nil {
130-		log.Printf("git: error flushing pkt-line message: %s", err)
131+		log.Debugf("git: error flushing pkt-line message: %s", err)
132 	}
133 }
134 
135+// ensureWithin ensures the given repo is within the repos directory.
136+func ensureWithin(reposDir string, repo string) error {
137+	repoDir := filepath.Join(reposDir, repo)
138+	absRepos, err := filepath.Abs(reposDir)
139+	if err != nil {
140+		log.Debugf("failed to get absolute path for repo: %s", err)
141+		return ErrSystemMalfunction
142+	}
143+	absRepo, err := filepath.Abs(repoDir)
144+	if err != nil {
145+		log.Debugf("failed to get absolute path for repos: %s", err)
146+		return ErrSystemMalfunction
147+	}
148+
149+	// ensure the repo is within the repos directory
150+	if !strings.HasPrefix(absRepo, absRepos) {
151+		log.Debugf("repo path is outside of repos directory: %s", absRepo)
152+		return ErrInvalidRepo
153+	}
154+
155+	return nil
156+}
157+
158 func fileExists(path string) (bool, error) {
159 	_, err := os.Stat(path)
160 	if err == nil {
161diff --git a/server/ssh.go b/server/ssh.go
162index 5a1752a4fbe92747787e224666070da295ed9df7..6f7898ca3f207c80b47ce7771784e5643aed5453 100644
163--- a/server/ssh.go
164+++ b/server/ssh.go
165@@ -2,7 +2,6 @@ package server
166 
167 import (
168 	"errors"
169-	"fmt"
170 	"path/filepath"
171 	"strings"
172 	"time"
173@@ -88,20 +87,21 @@ func (s *SSHServer) Middleware(cfg *config.Config) wish.Middleware {
174 				if len(cmd) >= 2 && strings.HasPrefix(cmd[0], "git") {
175 					gc := cmd[0]
176 					// repo should be in the form of "repo.git"
177-					repo := sanitizeRepoName(cmd[1])
178-					name := repo
179-					if strings.Contains(repo, "/") {
180-						log.Printf("invalid repo: %s", repo)
181-						sshFatal(s, fmt.Errorf("%s: %s", ErrInvalidRepo, "user repos not supported"))
182-						return
183-					}
184+					name := sanitizeRepoName(cmd[1])
185 					pk := s.PublicKey()
186 					access := cfg.Access.AccessLevel(name, pk)
187 					// git bare repositories should end in ".git"
188 					// https://git-scm.com/docs/gitrepository-layout
189-					repo = strings.TrimSuffix(repo, ".git") + ".git"
190+					repo := name + ".git"
191+
192 					// FIXME: determine repositories path
193-					repoDir := filepath.Join(cfg.DataPath, "repos", repo)
194+					reposDir := filepath.Join(cfg.DataPath, "repos")
195+					if err := ensureWithin(reposDir, repo); err != nil {
196+						sshFatal(s, err)
197+						return
198+					}
199+
200+					repoDir := filepath.Join(reposDir, repo)
201 					switch gc {
202 					case ReceivePackBin:
203 						if access < backend.ReadWriteAccess {