Fix insecure password recovery design (OWASP A04:2021)

owasp-top10-2021-apps/a4/super-recovery-password/src/api/routes/ChangePassword.go

@@ -4,7 +4,6 @@ import (
 	"api/database"
 	"api/services"
 	"api/types"
-	"fmt"
 	"net/http"
 
 	"github.com/dgrijalva/jwt-go"
@@ -14,9 +13,11 @@ import (
 func ChangePassword(c echo.Context) (err error) {
 	user := c.Get("user").(*jwt.Token)
 	claims := user.Claims.(*services.JwtCustomClaims)
-	if claims.Recovery != true {
-		return c.JSON(http.StatusOK, echo.Map{
-			"message": "invalid token",
+
+	// Tokens de recuperacao nao podem mais trocar senha (fail closed).
+	if claims.Recovery {
+		return c.JSON(http.StatusForbidden, echo.Map{
+			"message": "recovery tokens cannot be used to change passwords",
 		})
 	}
 
@@ -25,25 +26,16 @@ func ChangePassword(c echo.Context) (err error) {
 		return
 	}
 	if u.Password != u.RepeatPassword {
-		return c.JSON(http.StatusOK, echo.Map{
-			"message": "password don`t match",
+		return c.JSON(http.StatusBadRequest, echo.Map{
+			"message": "passwords don't match",
 		})
 	}
 
-	password := types.ChangePassword{
-		Password:       u.Password,
-		RepeatPassword: u.RepeatPassword,
-	}
-
-	err = database.ChangePassword(claims.Name, password.Password, password.RepeatPassword)
-	if err != nil {
-		fmt.Println(err)
-		return c.JSON(http.StatusOK, echo.Map{
+	if err = database.ChangePassword(claims.Name, u.Password, u.RepeatPassword); err != nil {
+		return c.JSON(http.StatusInternalServerError, echo.Map{
 			"message": "failed to change password",
 		})
 	}
 
-	return c.JSON(http.StatusOK, echo.Map{
-		"message": "success",
-	})
-}
+	return c.JSON(http.StatusOK, echo.Map{"message": "success"})
+}

owasp-top10-2021-apps/a4/super-recovery-password/src/api/routes/RecoveryPassword.go

@@ -1,45 +1,23 @@
 package routes
 
 import (
-	"api/database"
-	"api/services"
-	"api/types"
-	"fmt"
 	"net/http"
-	"strings"
 
 	"github.com/labstack/echo"
 )
 
-func RecoveryPassword(c echo.Context) (err error) {
-	u := new(types.RecoveryPasswordAnswers)
-	if err = c.Bind(u); err != nil {
-		return
-	}
-	u.Login = strings.ToLower(u.Login)
-	recoveryPasswordAnswers := types.RecoveryPasswordAnswers{
-		Login:        u.Login,
-		FirstAnswer:  u.FirstAnswer,
-		SecondAnswer: u.SecondAnswer,
-	}
-
-	answers, err := database.RecoveryPassword(recoveryPasswordAnswers.Login, recoveryPasswordAnswers.FirstAnswer, recoveryPasswordAnswers.SecondAnswer)
-	if err != nil {
-		return c.JSON(http.StatusConflict, echo.Map{"message": "incorrect answers!"})
-	}
-
-	if answers.FirstAnswer != recoveryPasswordAnswers.FirstAnswer || answers.SecondAnswer != recoveryPasswordAnswers.SecondAnswer {
-		return c.JSON(http.StatusConflict, echo.Map{"message": "incorrect answers!"})
-	}
-
-	token, err := services.GenerateJwt(recoveryPasswordAnswers.Login, true)
-	if err != nil {
-		return c.JSON(http.StatusUnauthorized, echo.Map{
-			"token": "Error to generate token.",
-		})
-	}
-	fmt.Println(token)
-	return c.JSON(http.StatusOK, echo.Map{
-		"token": token,
+// RecoveryPassword está desativaodo!
+//
+// O mecanismo anterior, baseado em perguntas de segurança de baixa segurança,
+// permitia enumeração de usuários, brute force das respostas e emissão de um
+// token de recuperação — levando a account takeover.
+//
+// Até que um método de recuperação forte e out-of-band seja implementado
+// (ex.: token de uso único enviado por e-mail/SMS verificado), este endpoint
+// não processa nenhuma tentativa de recuperação e retorna uma resposta neutra,
+// idêntica em qualquer situação, para não revelar informação alguma.
+func RecoveryPassword(c echo.Context) error {
+	return c.JSON(http.StatusServiceUnavailable, echo.Map{
+		"message": "password recovery is temporarily unavailable",
 	})
-}
+}

owasp-top10-2021-apps/a4/super-recovery-password/src/api/server.go

@@ -9,6 +9,8 @@ import (
 	"net/http"
 	"os"
 	"time"
+	"sync"
+
 
 	"github.com/labstack/echo"
 	"github.com/labstack/echo/middleware"
@@ -32,10 +34,9 @@ func main() {
 	}))
 
 	e.GET("/healthcheck", routes.Healthcheck)
-	e.POST("/userinfo", routes.UserInfo)
-	e.POST("/register", routes.Register)
-	e.POST("/login", routes.Login)
-	e.POST("/recovery", routes.RecoveryPassword)
+	e.POST("/register", routes.Register, newRateLimiter(5, time.Minute))
+	e.POST("/login", routes.Login, newRateLimiter(5, time.Minute))
+	e.POST("/recovery", routes.RecoveryPassword, newRateLimiter(5, time.Minute))
 
 	r := e.Group("/reset")
 	config := middleware.JWTConfig{
@@ -50,6 +51,37 @@ func main() {
 	e.Logger.Fatal(e.Start(":3000"))
 }
 
+
+func newRateLimiter(maxRequests int, window time.Duration) echo.MiddlewareFunc {
+	var mu sync.Mutex
+	hits := make(map[string][]time.Time)
+
+	return func(next echo.HandlerFunc) echo.HandlerFunc {
+		return func(c echo.Context) error {
+			ip := c.RealIP()
+			now := time.Now()
+
+			mu.Lock()
+			defer mu.Unlock()
+
+			var recent []time.Time
+			for _, t := range hits[ip] {
+				if now.Sub(t) < window {
+					recent = append(recent, t)
+				}
+			}
+			if len(recent) >= maxRequests {
+				return c.JSON(http.StatusTooManyRequests, echo.Map{
+					"message": "too many requests, try again later",
+				})
+			}
+			hits[ip] = append(recent, now)
+
+			return next(c)
+		}
+	}
+}
+
 func checkAPIrequirements() error {
 
 	if err := checkEnvVars(); err != nil {