Fix(🩹): error handling in CSRF token storage retrieval

middleware/csrf/csrf.go

@@ -18,8 +18,11 @@
 	ErrRefererNoMatch  = errors.New("referer does not match host and is not a trusted origin")
 	ErrOriginInvalid   = errors.New("origin invalid")
 	ErrOriginNoMatch   = errors.New("origin does not match host and is not a trusted origin")
-	errOriginNotFound  = errors.New("origin not supplied or is null") // internal error, will not be returned to the user
-	dummyValue         = []byte{'+'}
+	ErrNotGetStorage   = errors.New("csrf storage not found data")
+	ErrNotSetStorage   = errors.New("csrf storage not set data")
+
+	errOriginNotFound = errors.New("origin not supplied or is null") // internal error, will not be returned to the user
+	dummyValue        = []byte{'+'}
 )
 
 // Handler for CSRF middleware
@@ -102,10 +105,9 @@
 		switch c.Method() {
 		case fiber.MethodGet, fiber.MethodHead, fiber.MethodOptions, fiber.MethodTrace:
 			cookieToken := c.Cookies(cfg.CookieName)
-
 			if cookieToken != "" {
-				raw := getRawFromStorage(c, cookieToken, cfg, sessionManager, storageManager)
-
+				// In this case, handling error doesn't make sense because we have validations after the switch.
+				raw, _ := getRawFromStorage(c, cookieToken, cfg, sessionManager, storageManager)
 				if raw != nil {
 					token = cookieToken // Token is valid, safe to set it
 				}
@@ -148,9 +150,8 @@
 				return cfg.ErrorHandler(c, ErrTokenInvalid)
 			}
 
-			raw := getRawFromStorage(c, extractedToken, cfg, sessionManager, storageManager)
-
-			if raw == nil {
+			raw, err := getRawFromStorage(c, extractedToken, cfg, sessionManager, storageManager)
+			if err != nil || raw == nil {
 				// If token is not in storage, expire the cookie
 				expireCSRFCookie(c, cfg)
 				// and return an error
@@ -209,7 +210,7 @@
 
 // getRawFromStorage returns the raw value from the storage for the given token
 // returns nil if the token does not exist, is expired or is invalid
-func getRawFromStorage(c fiber.Ctx, token string, cfg Config, sessionManager *sessionManager, storageManager *storageManager) []byte {
+func getRawFromStorage(c fiber.Ctx, token string, cfg Config, sessionManager *sessionManager, storageManager *storageManager) (raw []byte, err error) {
 	if cfg.Session != nil {
 		return sessionManager.getRaw(c, token, dummyValue)
 	}
@@ -221,7 +222,7 @@
 	if cfg.Session != nil {
 		sessionManager.setRaw(c, token, dummyValue, cfg.Expiration)
 	} else {
 		storageManager.setRaw(token, dummyValue, cfg.Expiration)
 	}
 }
 
@@ -229,7 +230,7 @@
 	if cfg.Session != nil {
 		sessionManager.delRaw(c)
 	} else {
 		storageManager.delRaw(token)
 	}
 }
 

middleware/csrf/session_manager.go

@@ -26,20 +26,22 @@
 }
 
 // get token from session
-func (m *sessionManager) getRaw(c fiber.Ctx, key string, raw []byte) []byte {
+func (m *sessionManager) getRaw(c fiber.Ctx, key string, raw []byte) (rawToken []byte, err error) {
 	sess, err := m.session.Get(c)
 	if err != nil {
-		return nil
+		return nil, err
 	}
+
 	token, ok := sess.Get(m.key).(Token)
-	if ok {
-		if token.Expiration.Before(time.Now()) || key != token.Key || !compareTokens(raw, token.Raw) {
-			return nil
-		}
-		return token.Raw
+	if !ok {
+		return nil, ErrTokenInvalid
+	}
+
+	if token.Expiration.Before(time.Now()) || key != token.Key || !compareTokens(raw, token.Raw) {
+		return nil, ErrTokenInvalid
 	}
 
-	return nil
+	return token.Raw, nil
 }
 
 // set token in session

middleware/csrf/storage_manager.go

@@ -1,6 +1,7 @@
 package csrf
 
 import (
+	"fmt"
 	"sync"
 	"time"
 
@@ -40,31 +41,48 @@
 }
 
 // get raw data from storage or memory
-func (m *storageManager) getRaw(key string) []byte {
-	var raw []byte
+func (m *storageManager) getRaw(key string) (raw []byte, err error) {
 	if m.storage != nil {
-		raw, _ = m.storage.Get(key) //nolint:errcheck // TODO: Do not ignore error
+		raw, err = m.storage.Get(key)
+		if err != nil {
+			return nil, fmt.Errorf("%w: %s", ErrNotGetStorage, err.Error())
+		}
 	} else {
-		raw, _ = m.memory.Get(key).([]byte) //nolint:errcheck // TODO: Do not ignore error
+		var ok bool
+		raw, ok = m.memory.Get(key).([]byte)
+		if !ok {
+			return nil, ErrNotGetStorage
+		}
 	}
-	return raw
+
+	return raw, nil
 }
 
 // set data to storage or memory
-func (m *storageManager) setRaw(key string, raw []byte, exp time.Duration) {
+func (m *storageManager) setRaw(key string, raw []byte, exp time.Duration) (err error) {
 	if m.storage != nil {
-		_ = m.storage.Set(key, raw, exp) //nolint:errcheck // TODO: Do not ignore error
+		err = m.storage.Set(key, raw, exp)
+		if err != nil {
+			return fmt.Errorf("%w: %s", ErrNotSetStorage, err.Error())
+		}
 	} else {
 		// the key is crucial in crsf and sometimes a reference to another value which can be reused later(pool/unsafe values concept), so a copy is made here
 		m.memory.Set(utils.CopyString(key), raw, exp)
 	}
+
+	return nil
 }
 
 // delete data from storage or memory
-func (m *storageManager) delRaw(key string) {
+func (m *storageManager) delRaw(key string) (err error) {
 	if m.storage != nil {
-		_ = m.storage.Delete(key) //nolint:errcheck // TODO: Do not ignore error
+		err = m.storage.Delete(key)
+		if err != nil {
+			return fmt.Errorf("%w: %s", ErrNotSetStorage, err.Error())
+		}
 	} else {
 		m.memory.Delete(key)
 	}
+
+	return nil
 }