Skip to content

Commit

Permalink
Merge pull request from GHSA-94w9-97p3-p368
Browse files Browse the repository at this point in the history
* feat: improved csrf with session support

* fix: double submit cookie

* feat: add warning cookie extractor without session

* feat: add warning CsrfFromCookie SameSite

* fix: use byes.Equal instead

* fix: Overriden CookieName KeyLookup cookie:<name>

* feat: Create helpers.go

* feat: use compareTokens (constant time compare)

* feat: validate cookie to prevent token injection

* refactor: clean up csrf.go

* docs: update comment about Double Submit Cookie

* docs: update docs for CSRF changes

* feat: add DeleteToken

* refactor: no else

* test: add more tests

* refactor: re-order tests

* docs: update safe methods RCF add note

* test: add CSRF_Cookie_Injection_Exploit

* feat: add SingleUseToken config

* test: check for new token

* docs: use warning

* fix: always register type Token

* feat: use UUIDv4

* test: swap in UUIDv4 here too

* fix: raw token injection

* fix: merege error

* feat: Sentinel errors

* chore: rename test

* fix: url parse

* test: add path to referer

* test: add expiration tests

* docs: add cookie prefix note

* docs: fix typo

* docs: add warning for refer checks

* test: add referer edge cases

And call ctx.Request.Reset() and
ctx.Response.Reset() before re-using ctx.
  • Loading branch information
sixcolors authored Oct 16, 2023
1 parent d736d3a commit 8c3916d
Show file tree
Hide file tree
Showing 5 changed files with 266 additions and 26 deletions.
10 changes: 10 additions & 0 deletions docs/api/middleware/csrf.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,20 @@ When using this method, pre-sessions are required and will be created if a sessi

When using this middleware, it is recommended that you serve your pages over HTTPS, that the `CookieSecure` option is set to `true`, and that the `CookieSameSite` option is set to `Lax` or `Strict`. This will ensure that the cookie is only sent over HTTPS and that it is not sent on requests from external sites.

:::note
Cookie prefixes __Host- and __Secure- can be used to further secure the cookie. However, these prefixes are not supported by all browsers and there are some other limitations. See [MDN#Set-Cookie#cookie_prefixes](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#cookie_prefixes) for more information.

To use these prefixes, set the `CookieName` option to `__Host-csrf_` or `__Secure-csrf_`.
:::

### Referer Checking

For HTTPS requests, this middleware performs strict referer checking. This means that even if a subdomain can set or modify cookies on your domain, it can’t force a user to post to your application since that request won’t come from your own exact domain.

:::warning
Referer checking is required for https requests protected by CSRF. All modern browsers will automatically include the Referer header in requests, including those made with the JS Fetch API. However, if you are using this middleware with a custom client you must ensure that the client sends a valid Referer header.
:::

### Token Lifecycle

Tokens are valid until they expire, or until they are deleted. By default, tokens are valid for 1 hour and each subsequent request will extend the expiration by 1 hour. This means that if a user makes a request every hour, the token will never expire. If a user makes a request after the token has expired, then a new token will be generated and the `csrf_` cookie will be set again. This means that the token will only expire if the user does not make a request for the duration of the expiration time.
Expand Down
28 changes: 18 additions & 10 deletions middleware/csrf/csrf.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package csrf

import (
"errors"
"net/url"
"reflect"
"time"

Expand Down Expand Up @@ -63,10 +64,10 @@ func New(config ...Config) fiber.Handler {
cookieToken := c.Cookies(cfg.CookieName)

if cookieToken != "" {
rawToken := getTokenFromStorage(c, cookieToken, cfg, sessionManager, storageManager)
raw := getRawFromStorage(c, cookieToken, cfg, sessionManager, storageManager)

if rawToken != nil {
token = string(rawToken)
if raw != nil {
token = cookieToken // Token is valid, safe to set it
}
}
default:
Expand All @@ -92,13 +93,13 @@ func New(config ...Config) fiber.Handler {
// If not using CsrfFromCookie extractor, check that the token matches the cookie
// This is to prevent CSRF attacks by using a Double Submit Cookie method
// Useful when we do not have access to the users Session
if !isCsrfFromCookie(cfg.Extractor) && extractedToken != c.Cookies(cfg.CookieName) {
if !isCsrfFromCookie(cfg.Extractor) && !compareStrings(extractedToken, c.Cookies(cfg.CookieName)) {
return cfg.ErrorHandler(c, ErrTokenInvalid)
}

rawToken := getTokenFromStorage(c, extractedToken, cfg, sessionManager, storageManager)
raw := getRawFromStorage(c, extractedToken, cfg, sessionManager, storageManager)

if rawToken == nil {
if raw == nil {
// If token is not in storage, expire the cookie
expireCSRFCookie(c, cfg)
// and return an error
Expand All @@ -108,7 +109,7 @@ func New(config ...Config) fiber.Handler {
// If token is single use, delete it from storage
deleteTokenFromStorage(c, extractedToken, cfg, sessionManager, storageManager)
} else {
token = string(rawToken)
token = extractedToken // Token is valid, safe to set it
}
}

Expand Down Expand Up @@ -137,9 +138,9 @@ func New(config ...Config) fiber.Handler {
}
}

// getTokenFromStorage returns the raw token from the storage
// 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 getTokenFromStorage(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) []byte {
if cfg.Session != nil {
return sessionManager.getRaw(c, token, dummyValue)
}
Expand Down Expand Up @@ -223,8 +224,15 @@ func refererMatchesHost(c *fiber.Ctx) error {
if referer == "" {
return ErrNoReferer
}
if referer != c.Protocol()+"://"+c.Hostname() {

refererURL, err := url.Parse(referer)
if err != nil {
return ErrBadReferer
}

if refererURL.Scheme+"://"+refererURL.Host != c.Protocol()+"://"+c.Hostname() {
return ErrBadReferer
}

return nil
}
Loading

0 comments on commit 8c3916d

Please sign in to comment.