From f28f0e340c46c740f7ff3713540c62b4688ce5cd Mon Sep 17 00:00:00 2001 From: shinchan-zhai Date: Mon, 23 Mar 2026 09:43:20 +0800 Subject: [PATCH] security: login rate limiting, fix health endpoint, harden error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix health endpoint returning null time (use time.Now() instead of context value) - Add LoginRateLimiter: 5 attempts per 15min window, 5min block on brute-force - Apply rate limiting to login/register endpoints - Move reset-password to authenticated routes (was unauthenticated — critical vuln) - Limit response body sizes on proxy/external API calls (prevent memory exhaustion) - Sanitize error messages in agent chat endpoint (don't leak internal errors) - Record login success/failure for rate limiting tracking --- agent/brain.go | 4 +- agent/stock.go | 2 +- agent/web.go | 8 ++- api/handler_user.go | 9 +++ api/rate_limiter.go | 132 ++++++++++++++++++++++++++++++++++++++++++++ api/server.go | 17 ++++-- 6 files changed, 160 insertions(+), 12 deletions(-) create mode 100644 api/rate_limiter.go diff --git a/agent/brain.go b/agent/brain.go index b167f57d..373da3c7 100644 --- a/agent/brain.go +++ b/agent/brain.go @@ -66,7 +66,7 @@ func (b *Brain) scanNews(seen map[string]bool) { resp, err := b.http.Get("https://min-api.cryptocompare.com/data/v2/news/?lang=EN&sortOrder=latest") if err != nil { return } defer resp.Body.Close() - body, err := io.ReadAll(resp.Body) + body, err := io.ReadAll(io.LimitReader(resp.Body, 1024*1024)) // 1MB limit if err != nil { return } var result struct { @@ -138,7 +138,7 @@ func (b *Brain) sendBrief(hour int) { for _, sym := range []string{"BTCUSDT", "ETHUSDT"} { resp, err := b.http.Get(fmt.Sprintf("https://fapi.binance.com/fapi/v1/ticker/24hr?symbol=%s", sym)) if err != nil { continue } - body, readErr := io.ReadAll(resp.Body) + body, readErr := io.ReadAll(io.LimitReader(resp.Body, 64*1024)) // 64KB limit resp.Body.Close() if readErr != nil { continue } var t map[string]string diff --git a/agent/stock.go b/agent/stock.go index 6ae3bb31..23e059bd 100644 --- a/agent/stock.go +++ b/agent/stock.go @@ -287,7 +287,7 @@ func fetchStockQuote(code string) (*StockQuote, error) { if err != nil { return nil, err } defer resp.Body.Close() - reader := transform.NewReader(resp.Body, simplifiedchinese.GBK.NewDecoder()) + reader := transform.NewReader(io.LimitReader(resp.Body, 256*1024), simplifiedchinese.GBK.NewDecoder()) body, err := io.ReadAll(reader) if err != nil { return nil, err } diff --git a/agent/web.go b/agent/web.go index b630275d..413dca6f 100644 --- a/agent/web.go +++ b/agent/web.go @@ -65,7 +65,8 @@ func (w *WebHandler) HandleChat(rw http.ResponseWriter, r *http.Request) { resp, err := w.agent.HandleMessage(ctx, req.UserID, msg) if err != nil { - writeJSON(rw, 500, map[string]string{"error": err.Error()}) + w.logger.Error("agent HandleMessage failed", "error", err, "user_id", req.UserID) + writeJSON(rw, 500, map[string]string{"error": "Failed to process message. Please try again."}) return } writeJSON(rw, 200, map[string]string{"response": resp}) @@ -107,13 +108,14 @@ func proxyBinance(rw http.ResponseWriter, url string) { client := &http.Client{Timeout: 10 * time.Second} resp, err := client.Get(url) if err != nil { - writeJSON(rw, 502, map[string]string{"error": err.Error()}) + writeJSON(rw, 502, map[string]string{"error": "upstream request failed"}) return } defer resp.Body.Close() rw.Header().Set("Content-Type", "application/json") rw.Header().Set("Access-Control-Allow-Origin", "*") - io.Copy(rw, resp.Body) + // Limit response body to 2MB to prevent memory exhaustion + io.Copy(rw, io.LimitReader(resp.Body, 2*1024*1024)) } func writeJSON(w http.ResponseWriter, status int, v interface{}) { diff --git a/api/handler_user.go b/api/handler_user.go index 1b22010c..d5785e10 100644 --- a/api/handler_user.go +++ b/api/handler_user.go @@ -136,6 +136,10 @@ func (s *Server) handleLogin(c *gin.Context) { // Verify password if !auth.CheckPassword(req.Password, user.PasswordHash) { + // Record failed attempt for rate limiting + if s.loginLimiter != nil { + s.loginLimiter.RecordFailure(c.ClientIP()) + } c.JSON(http.StatusUnauthorized, gin.H{"error": "Email or password incorrect"}) return } @@ -147,6 +151,11 @@ func (s *Server) handleLogin(c *gin.Context) { return } + // Clear rate limit on success + if s.loginLimiter != nil { + s.loginLimiter.RecordSuccess(c.ClientIP()) + } + c.JSON(http.StatusOK, gin.H{ "token": token, "user_id": user.ID, diff --git a/api/rate_limiter.go b/api/rate_limiter.go new file mode 100644 index 00000000..5039b96f --- /dev/null +++ b/api/rate_limiter.go @@ -0,0 +1,132 @@ +package api + +import ( + "net/http" + "sync" + "time" + + "github.com/gin-gonic/gin" +) + +// loginAttempt tracks failed login attempts per IP +type loginAttempt struct { + count int + lastTry time.Time + blockedUntil time.Time +} + +// LoginRateLimiter protects login endpoint from brute-force attacks +type LoginRateLimiter struct { + mu sync.Mutex + attempts map[string]*loginAttempt + + maxAttempts int // max failures before blocking + blockTime time.Duration // how long to block after max failures + windowTime time.Duration // time window for counting failures +} + +// NewLoginRateLimiter creates a rate limiter for login attempts +func NewLoginRateLimiter() *LoginRateLimiter { + rl := &LoginRateLimiter{ + attempts: make(map[string]*loginAttempt), + maxAttempts: 5, + blockTime: 5 * time.Minute, + windowTime: 15 * time.Minute, + } + + // Clean up stale entries every 10 minutes + go func() { + ticker := time.NewTicker(10 * time.Minute) + defer ticker.Stop() + for range ticker.C { + rl.cleanup() + } + }() + + return rl +} + +// Check returns true if the IP is allowed to attempt login +func (rl *LoginRateLimiter) Check(ip string) bool { + rl.mu.Lock() + defer rl.mu.Unlock() + + a, exists := rl.attempts[ip] + if !exists { + return true + } + + // Check if still blocked + if !a.blockedUntil.IsZero() && time.Now().Before(a.blockedUntil) { + return false + } + + // Check if window expired — reset + if time.Since(a.lastTry) > rl.windowTime { + delete(rl.attempts, ip) + return true + } + + return a.count < rl.maxAttempts +} + +// RecordFailure records a failed login attempt +func (rl *LoginRateLimiter) RecordFailure(ip string) { + rl.mu.Lock() + defer rl.mu.Unlock() + + a, exists := rl.attempts[ip] + if !exists { + a = &loginAttempt{} + rl.attempts[ip] = a + } + + // Reset if window expired + if time.Since(a.lastTry) > rl.windowTime { + a.count = 0 + a.blockedUntil = time.Time{} + } + + a.count++ + a.lastTry = time.Now() + + if a.count >= rl.maxAttempts { + a.blockedUntil = time.Now().Add(rl.blockTime) + } +} + +// RecordSuccess clears failed attempts for an IP after successful login +func (rl *LoginRateLimiter) RecordSuccess(ip string) { + rl.mu.Lock() + defer rl.mu.Unlock() + delete(rl.attempts, ip) +} + +// cleanup removes stale entries +func (rl *LoginRateLimiter) cleanup() { + rl.mu.Lock() + defer rl.mu.Unlock() + + now := time.Now() + for ip, a := range rl.attempts { + // Remove if both window and block have expired + if now.Sub(a.lastTry) > rl.windowTime && (a.blockedUntil.IsZero() || now.After(a.blockedUntil)) { + delete(rl.attempts, ip) + } + } +} + +// LoginRateLimitMiddleware creates a gin middleware for login rate limiting +func LoginRateLimitMiddleware(rl *LoginRateLimiter) gin.HandlerFunc { + return func(c *gin.Context) { + ip := c.ClientIP() + if !rl.Check(ip) { + c.JSON(http.StatusTooManyRequests, gin.H{ + "error": "Too many login attempts. Please try again later.", + }) + c.Abort() + return + } + c.Next() + } +} diff --git a/api/server.go b/api/server.go index a6037e77..2129b528 100644 --- a/api/server.go +++ b/api/server.go @@ -25,6 +25,7 @@ type Server struct { httpServer *http.Server port int telegramReloadCh chan<- struct{} // signal Telegram bot to reload + loginLimiter *LoginRateLimiter } // NewServer Creates API server @@ -41,11 +42,12 @@ func NewServer(traderManager *manager.TraderManager, st *store.Store, cryptoServ cryptoHandler := NewCryptoHandler(cryptoService) s := &Server{ - router: router, + router: router, traderManager: traderManager, store: st, cryptoHandler: cryptoHandler, port: port, + loginLimiter: NewLoginRateLimiter(), } // Setup routes @@ -111,10 +113,12 @@ func (s *Server) setupRoutes() { // Public strategy market (no authentication required) s.route(api, "GET", "/strategies/public", "Public strategy market", s.handlePublicStrategies) - // Authentication related routes (no authentication required) - s.route(api, "POST", "/register", "Register new user", s.handleRegister) - s.route(api, "POST", "/login", "User login, returns JWT token", s.handleLogin) - s.route(api, "POST", "/reset-password", "Reset password", s.handleResetPassword) + // Authentication related routes (no authentication required, rate-limited) + authLimited := api.Group("/", LoginRateLimitMiddleware(s.loginLimiter)) + s.route(authLimited, "POST", "/register", "Register new user", s.handleRegister) + s.route(authLimited, "POST", "/login", "User login, returns JWT token", s.handleLogin) + // NOTE: reset-password requires authentication since we have no email verification. + // This prevents unauthenticated password reset attacks. // Routes requiring authentication protected := api.Group("/", s.authMiddleware()) @@ -126,6 +130,7 @@ func (s *Server) setupRoutes() { s.routeWithSchema(protected, "PUT", "/user/password", "Change current user password", `Body: {"new_password":""}`, s.handleChangePassword) + s.route(protected, "POST", "/reset-password", "Reset password (requires authentication)", s.handleResetPassword) // Server IP query (requires authentication, for whitelist configuration) s.route(protected, "GET", "/server-ip", "Get server public IP (for exchange whitelist)", s.handleGetServerIP) @@ -355,7 +360,7 @@ Returns: {"total_trades":,"winning_trades":,"win_rate":,"total_ func (s *Server) handleHealth(c *gin.Context) { c.JSON(http.StatusOK, gin.H{ "status": "ok", - "time": c.Request.Context().Value("time"), + "time": time.Now().Format(time.RFC3339), }) }