fix(security): harden proxy trust, lockouts and hooks
- commit
af5ee8972884b21f9998e87226fd0050ffcd5db7browse tree- parent
ca3bf534ca0980572c3ad1bea3b7fe2b06172952- author
- obscurejames <354576047+obscurejames@users.noreply.github.com>
- signature
- Signed by:
~james SHA256:KlzJ0P0gvJcnOYpaxD5yCUw52ImHttTKRjVAEnnYnKg
- date
- 2026-10-20 20:35 UTC
10 files changed, +80 −19
- account.go modified +3 −2
- avatar.go modified +7 −2
- edit.go modified +4 −2
- extra.go modified +6 −0
- git.go modified +31 −1
- limit.go modified +7 −5
- main.go modified +2 −0
- search.go modified +14 −4
- smart.go modified +2 −1
- web.go modified +4 −2
account.go modified
| @@ -135,8 +135,8 @@ func (s *Server) totpLogin(w http.ResponseWriter, r *http.Request) { | ||
| 135 | 135 | http.Redirect(w, r, "/login", http.StatusSeeOther) |
| 136 | 136 | return |
| 137 | 137 | } |
| 138 | - ip := clientIP(r) | |
| 139 | - if s.limiter.blocked(ip) { | |
| 138 | + ip, who := clientIP(r), "totp:"+u.Name | |
| 139 | + if s.limiter.blocked(ip) || s.limiter.blocked(who) { | |
| 140 | 140 | s.fail(w, r, 429, "Too many attempts, try again in a few minutes") |
| 141 | 141 | return |
| 142 | 142 | } |
| @@ -147,6 +147,7 @@ func (s *Server) totpLogin(w http.ResponseWriter, r *http.Request) { | ||
| 147 | 147 | s.store.SetTOTP(u.ID, a.TOTP, left) |
| 148 | 148 | } else { |
| 149 | 149 | s.limiter.fail(ip) |
| 150 | + s.limiter.fail(who) | |
| 150 | 151 | w.WriteHeader(401) |
| 151 | 152 | s.render(w, r, "totp", page{Title: "Two factor", Error: "Wrong code"}) |
| 152 | 153 | return |
avatar.go modified
| @@ -7,6 +7,7 @@ import ( | ||
| 7 | 7 | _ "image/gif" |
| 8 | 8 | _ "image/jpeg" |
| 9 | 9 | "image/png" |
| 10 | + "sync" | |
| 10 | 11 | |
| 11 | 12 | "golang.org/x/image/draw" |
| 12 | 13 | _ "golang.org/x/image/webp" |
| @@ -14,14 +15,18 @@ import ( | ||
| 14 | 15 | |
| 15 | 16 | const avatarSize = 256 |
| 16 | 17 | |
| 18 | +var decoding sync.Mutex | |
| 19 | + | |
| 17 | 20 | func cropAvatar(data []byte, x, y, size int) ([]byte, error) { |
| 18 | 21 | cfg, _, err := image.DecodeConfig(bytes.NewReader(data)) |
| 19 | 22 | if err != nil { |
| 20 | 23 | return nil, errors.New("Avatar must be a PNG, JPEG, GIF or WebP image") |
| 21 | 24 | } |
| 22 | - if cfg.Width*cfg.Height > 25_000_000 { | |
| 23 | - return nil, errors.New("Avatar image is too large, keep it under 25 megapixels") | |
| 25 | + if cfg.Width > 6000 || cfg.Height > 6000 || cfg.Width*cfg.Height > 16_000_000 { | |
| 26 | + return nil, errors.New("Avatar image is too large, keep it under 16 megapixels") | |
| 24 | 27 | } |
| 28 | + decoding.Lock() | |
| 29 | + defer decoding.Unlock() | |
| 25 | 30 | img, _, err := image.Decode(bytes.NewReader(data)) |
| 26 | 31 | if err != nil { |
| 27 | 32 | return nil, errors.New("Avatar must be a PNG, JPEG, GIF or WebP image") |
edit.go modified
| @@ -49,11 +49,13 @@ func hook(root, kind, spec string) int { | ||
| 49 | 49 | name := strings.TrimSuffix(repoDir, ".git") |
| 50 | 50 | store, err := openStore(filepath.Join(root, "gitpocket.db"), true) |
| 51 | 51 | if err != nil { |
| 52 | - return 0 | |
| 52 | + fmt.Fprintln(os.Stderr, "gitpocket: database unavailable, push refused") | |
| 53 | + return 1 | |
| 53 | 54 | } |
| 54 | 55 | repo, err := store.RepoByPath(owner, name) |
| 55 | 56 | if err != nil { |
| 56 | - return 0 | |
| 57 | + fmt.Fprintln(os.Stderr, "gitpocket: unknown repository, push refused") | |
| 58 | + return 1 | |
| 57 | 59 | } |
| 58 | 60 | for _, kv := range os.Environ() { |
| 59 | 61 | if strings.HasPrefix(kv, "GIT_OBJECT_DIRECTORY=") || strings.HasPrefix(kv, "GIT_ALTERNATE_OBJECT_DIRECTORIES=") || strings.HasPrefix(kv, "GIT_QUARANTINE_PATH=") { |
extra.go modified
| @@ -282,6 +282,12 @@ func (s *Server) download(w http.ResponseWriter, r *http.Request, repo Repo, spe | ||
| 282 | 282 | } |
| 283 | 283 | |
| 284 | 284 | func writeArchive(ctx context.Context, w io.Writer, dir, format, name, commit string) error { |
| 285 | + select { | |
| 286 | + case archiveSlots <- struct{}{}: | |
| 287 | + case <-ctx.Done(): | |
| 288 | + return ctx.Err() | |
| 289 | + } | |
| 290 | + defer func() { <-archiveSlots }() | |
| 285 | 291 | gitFormat := format |
| 286 | 292 | if format == "tar.gz" { |
| 287 | 293 | gitFormat = "tar" |
git.go modified
| @@ -8,6 +8,7 @@ import ( | ||
| 8 | 8 | "os" |
| 9 | 9 | "os/exec" |
| 10 | 10 | "path" |
| 11 | + "runtime" | |
| 11 | 12 | "slices" |
| 12 | 13 | "strconv" |
| 13 | 14 | "strings" |
| @@ -80,7 +81,30 @@ func (g Git) run(stdin []byte, args ...string) ([]byte, error) { | ||
| 80 | 81 | return g.runEnv(nil, stdin, args...) |
| 81 | 82 | } |
| 82 | 83 | |
| 84 | +const maxOutput = 16 << 20 | |
| 85 | + | |
| 86 | +var ( | |
| 87 | + gitSlots = make(chan struct{}, max(4, 2*runtime.NumCPU())) | |
| 88 | + archiveSlots = make(chan struct{}, 2) | |
| 89 | + errTooLarge = errors.New("output too large") | |
| 90 | +) | |
| 91 | + | |
| 92 | +type cappedBuffer struct { | |
| 93 | + bytes.Buffer | |
| 94 | + over bool | |
| 95 | +} | |
| 96 | + | |
| 97 | +func (b *cappedBuffer) Write(p []byte) (int, error) { | |
| 98 | + if b.Len()+len(p) > maxOutput { | |
| 99 | + b.over = true | |
| 100 | + return 0, errTooLarge | |
| 101 | + } | |
| 102 | + return b.Buffer.Write(p) | |
| 103 | +} | |
| 104 | + | |
| 83 | 105 | func (g Git) runEnv(env []string, stdin []byte, args ...string) ([]byte, error) { |
| 106 | + gitSlots <- struct{}{} | |
| 107 | + defer func() { <-gitSlots }() | |
| 84 | 108 | ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) |
| 85 | 109 | defer cancel() |
| 86 | 110 | cmd := exec.CommandContext(ctx, "git", append([]string{"-C", g.Dir}, args...)...) |
| @@ -95,10 +119,14 @@ func (g Git) runEnv(env []string, stdin []byte, args ...string) ([]byte, error) | ||
| 95 | 119 | if stdin != nil { |
| 96 | 120 | cmd.Stdin = bytes.NewReader(stdin) |
| 97 | 121 | } |
| 98 | - var out, errb bytes.Buffer | |
| 122 | + var out cappedBuffer | |
| 123 | + var errb bytes.Buffer | |
| 99 | 124 | cmd.Stdout = &out |
| 100 | 125 | cmd.Stderr = &errb |
| 101 | 126 | if err := cmd.Run(); err != nil { |
| 127 | + if out.over { | |
| 128 | + return nil, errTooLarge | |
| 129 | + } | |
| 102 | 130 | msg := strings.TrimSpace(errb.String()) |
| 103 | 131 | if msg == "" { |
| 104 | 132 | msg = err.Error() |
| @@ -486,6 +514,8 @@ func (g Git) LastCommits(commit, dir string, names []string) map[string]Commit { | ||
| 486 | 514 | } |
| 487 | 515 | |
| 488 | 516 | func (g Git) Stream(w io.Writer, commit, filePath string) error { |
| 517 | + archiveSlots <- struct{}{} | |
| 518 | + defer func() { <-archiveSlots }() | |
| 489 | 519 | ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) |
| 490 | 520 | defer cancel() |
| 491 | 521 | cmd := exec.CommandContext(ctx, "git", "-C", g.Dir, "cat-file", "blob", commit+":"+filePath) |
limit.go modified
| @@ -56,13 +56,15 @@ func (l *limiter) fail(key string) { | ||
| 56 | 56 | } |
| 57 | 57 | |
| 58 | 58 | func clientIP(r *http.Request) string { |
| 59 | - if xff := r.Header.Get("X-Forwarded-For"); xff != "" { | |
| 60 | - first, _, _ := strings.Cut(xff, ",") | |
| 61 | - return strings.TrimSpace(first) | |
| 62 | - } | |
| 63 | 59 | host, _, err := net.SplitHostPort(r.RemoteAddr) |
| 64 | 60 | if err != nil { |
| 65 | - return r.RemoteAddr | |
| 61 | + host = r.RemoteAddr | |
| 62 | + } | |
| 63 | + if ip := net.ParseIP(host); ip != nil && ip.IsLoopback() { | |
| 64 | + if xff := r.Header.Get("X-Forwarded-For"); xff != "" { | |
| 65 | + hops := strings.Split(xff, ",") | |
| 66 | + return strings.TrimSpace(hops[len(hops)-1]) | |
| 67 | + } | |
| 66 | 68 | } |
| 67 | 69 | return host |
| 68 | 70 | } |
main.go modified
| @@ -206,6 +206,8 @@ func serve(root string, args []string) { | ||
| 206 | 206 | Addr: *listen, |
| 207 | 207 | Handler: newServer(store, root, *host, *name, *open, *sshUser).handler(), |
| 208 | 208 | ReadHeaderTimeout: 10 * time.Second, |
| 209 | + IdleTimeout: 2 * time.Minute, | |
| 210 | + MaxHeaderBytes: 64 << 10, | |
| 209 | 211 | } |
| 210 | 212 | log.Printf("gitpocket listening on %s for %s", *listen, *host) |
| 211 | 213 | log.Fatal(srv.ListenAndServe()) |
search.go modified
| @@ -7,6 +7,7 @@ import ( | ||
| 7 | 7 | "strconv" |
| 8 | 8 | "strings" |
| 9 | 9 | "sync" |
| 10 | + "time" | |
| 10 | 11 | ) |
| 11 | 12 | |
| 12 | 13 | type language struct { |
| @@ -48,6 +49,7 @@ func vendored(name string) bool { | ||
| 48 | 49 | |
| 49 | 50 | type langEntry struct { |
| 50 | 51 | head string |
| 52 | + stamp time.Time | |
| 51 | 53 | langs []language |
| 52 | 54 | } |
| 53 | 55 | |
| @@ -58,14 +60,22 @@ var langCache = struct { | ||
| 58 | 60 | |
| 59 | 61 | func (s *Server) repoLanguages(repo Repo) []language { |
| 60 | 62 | g := Git{Dir: s.repoDir(repo)} |
| 63 | + stamp := repoUpdated(g.Dir) | |
| 64 | + langCache.Lock() | |
| 65 | + e, ok := langCache.m[repo.ID] | |
| 66 | + langCache.Unlock() | |
| 67 | + if ok && e.stamp.Equal(stamp) { | |
| 68 | + return e.langs | |
| 69 | + } | |
| 61 | 70 | head, err := g.Resolve("HEAD") |
| 62 | 71 | if err != nil { |
| 63 | 72 | return nil |
| 64 | 73 | } |
| 65 | - langCache.Lock() | |
| 66 | - e, ok := langCache.m[repo.ID] | |
| 67 | - langCache.Unlock() | |
| 68 | 74 | if ok && e.head == head { |
| 75 | + e.stamp = stamp | |
| 76 | + langCache.Lock() | |
| 77 | + langCache.m[repo.ID] = e | |
| 78 | + langCache.Unlock() | |
| 69 | 79 | return e.langs |
| 70 | 80 | } |
| 71 | 81 | out, err := g.run(nil, "ls-tree", "-r", "-l", "-z", head) |
| @@ -105,7 +115,7 @@ func (s *Server) repoLanguages(repo Repo) []language { | ||
| 105 | 115 | return strings.Compare(a.Name, b.Name) |
| 106 | 116 | }) |
| 107 | 117 | langCache.Lock() |
| 108 | - langCache.m[repo.ID] = langEntry{head, langs} | |
| 118 | + langCache.m[repo.ID] = langEntry{head, stamp, langs} | |
| 109 | 119 | langCache.Unlock() |
| 110 | 120 | return langs |
| 111 | 121 | } |
smart.go modified
| @@ -6,6 +6,7 @@ import ( | ||
| 6 | 6 | "fmt" |
| 7 | 7 | "io" |
| 8 | 8 | "net/http" |
| 9 | + "os" | |
| 9 | 10 | "os/exec" |
| 10 | 11 | "strings" |
| 11 | 12 | "time" |
| @@ -101,7 +102,7 @@ func (s *Server) smart(w http.ResponseWriter, r *http.Request, owner User, name, | ||
| 101 | 102 | } |
| 102 | 103 | cmd := exec.CommandContext(ctx, "git", strings.TrimPrefix(service, "git-"), "--stateless-rpc", dir) |
| 103 | 104 | cmd.Stdin = body |
| 104 | - cmd.Env = []string{"GIT_PROTOCOL=" + r.Header.Get("Git-Protocol"), "HOME=/nonexistent"} | |
| 105 | + cmd.Env = []string{"GIT_PROTOCOL=" + r.Header.Get("Git-Protocol"), "HOME=/nonexistent", "PATH=" + os.Getenv("PATH")} | |
| 105 | 106 | stdout, err := cmd.StdoutPipe() |
| 106 | 107 | if err != nil { |
| 107 | 108 | http.Error(w, "git failed", 500) |
web.go modified
| @@ -260,14 +260,16 @@ func (s *Server) loginForm(w http.ResponseWriter, r *http.Request) { | ||
| 260 | 260 | |
| 261 | 261 | func (s *Server) login(w http.ResponseWriter, r *http.Request) { |
| 262 | 262 | ip := clientIP(r) |
| 263 | - if s.limiter.blocked(ip) { | |
| 263 | + name := strings.TrimSpace(r.FormValue("name")) | |
| 264 | + if s.limiter.blocked(ip) || s.limiter.blocked("~"+name) { | |
| 264 | 265 | w.WriteHeader(429) |
| 265 | 266 | s.render(w, r, "login", page{Title: "Log in", Error: "Too many attempts, try again in a few minutes"}) |
| 266 | 267 | return |
| 267 | 268 | } |
| 268 | - u, ok := s.store.CheckPassword(strings.TrimSpace(r.FormValue("name")), r.FormValue("password")) | |
| 269 | + u, ok := s.store.CheckPassword(name, r.FormValue("password")) | |
| 269 | 270 | if !ok { |
| 270 | 271 | s.limiter.fail(ip) |
| 272 | + s.limiter.fail("~" + name) | |
| 271 | 273 | w.WriteHeader(401) |
| 272 | 274 | s.render(w, r, "login", page{Title: "Log in", Error: "Wrong name or password"}) |
| 273 | 275 | return |