Skip to content

Commit

Permalink
Merge pull request from GHSA-fmg4-x8pw-hjhg
Browse files Browse the repository at this point in the history 
* Enforce Wildcard Origins with AllowCredentials check

* Expand unit-tests, fix issues with subdomains logic, update docs

* Update cors.md

* Added test using localhost, ipv4, and ipv6 address

* improve documentation markdown

---------

Co-authored-by: René Werner <rene@gofiber.io>
  • Loading branch information
@gaby @ReneWerner87
gaby and ReneWerner87 committed Feb 21, 2024
1 parent 5e30112 commit f0cd3b4
Show file tree
Hide file tree
Showing 5 changed files with 402 additions and 88 deletions.
31 changes: 21 additions & 10 deletions docs/api/middleware/cors.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ The middleware conforms to the `access-control-allow-origin` specification by pa

For more control, `AllowOriginsFunc` can be used to programatically determine if an origin is allowed. If no match was found in `AllowOrigins` and if `AllowOriginsFunc` returns true then the 'access-control-allow-origin' response header is set to the 'origin' request header.

When defining your Origins make sure they are properly formatted. The middleware validates and normalizes the provided origins, ensuring they're in the correct format by checking for valid schemes (http or https), and removing any trailing slashes.

## Signatures

```go
Expand Down Expand Up @@ -56,18 +58,27 @@ app.Use(cors.New(cors.Config{
}))
```

**Note: The following configuration is considered insecure and will result in a panic.**

```go
app.Use(cors.New(cors.Config{
AllowOrigins: "*",
AllowCredentials: true,
}))
```

## Config

| Property | Type | Description | Default |
|:-----------------|:---------------------------|:----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:-----------------------------------|
| Next | `func(*fiber.Ctx) bool` | Next defines a function to skip this middleware when returned true. | `nil` |
| AllowOriginsFunc | `func(origin string) bool` | AllowOriginsFunc defines a function that will set the 'access-control-allow-origin' response header to the 'origin' request header when returned true. | `nil` |
| AllowOrigins | `string` | AllowOrigin defines a comma separated list of origins that may access the resource. | `"*"` |
| AllowMethods | `string` | AllowMethods defines a list of methods allowed when accessing the resource. This is used in response to a preflight request. | `"GET,POST,HEAD,PUT,DELETE,PATCH"` |
| AllowHeaders | `string` | AllowHeaders defines a list of request headers that can be used when making the actual request. This is in response to a preflight request. | `""` |
| AllowCredentials | `bool` | AllowCredentials indicates whether or not the response to the request can be exposed when the credentials flag is true. | `false` |
| ExposeHeaders | `string` | ExposeHeaders defines a whitelist headers that clients are allowed to access. | `""` |
| MaxAge | `int` | MaxAge indicates how long (in seconds) the results of a preflight request can be cached. If you pass MaxAge 0, Access-Control-Max-Age header will not be added and browser will use 5 seconds by default. To disable caching completely, pass MaxAge value negative. It will set the Access-Control-Max-Age header 0. | `0` |
| Property | Type | Description | Default |
|:-----------------|:---------------------------|:-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:-----------------------------------|
| Next | `func(*fiber.Ctx) bool` | Next defines a function to skip this middleware when returned true. | `nil` |
| AllowOriginsFunc | `func(origin string) bool` | AllowOriginsFunc defines a function that will set the 'access-control-allow-origin' response header to the 'origin' request header when returned true. This allows for dynamic evaluation of allowed origins. Note if AllowCredentials is true, wildcard origins will be not have the 'access-control-allow-credentials' header set to 'true'. | `nil` |
| AllowOrigins | `string` | AllowOrigin defines a comma separated list of origins that may access the resource. | `"*"` |
| AllowMethods | `string` | AllowMethods defines a list of methods allowed when accessing the resource. This is used in response to a preflight request. | `"GET,POST,HEAD,PUT,DELETE,PATCH"` |
| AllowHeaders | `string` | AllowHeaders defines a list of request headers that can be used when making the actual request. This is in response to a preflight request. | `""` |
| AllowCredentials | `bool` | AllowCredentials indicates whether or not the response to the request can be exposed when the credentials flag is true. When used as part of a response to a preflight request, this indicates whether or not the actual request can be made using credentials. Note: If true, AllowOrigins cannot be set to a wildcard ("*") to prevent security vulnerabilities. | `false` |
| ExposeHeaders | `string` | ExposeHeaders defines a whitelist headers that clients are allowed to access. | `""` |
| MaxAge | `int` | MaxAge indicates how long (in seconds) the results of a preflight request can be cached. If you pass MaxAge 0, Access-Control-Max-Age header will not be added and browser will use 5 seconds by default. To disable caching completely, pass MaxAge value negative. It will set the Access-Control-Max-Age header 0. | `0` |

## Default Config

Expand Down
61 changes: 44 additions & 17 deletions middleware/cors/cors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@ type Config struct {
Next func(c *fiber.Ctx) bool

// AllowOriginsFunc defines a function that will set the 'access-control-allow-origin'
// response header to the 'origin' request header when returned true.
// response header to the 'origin' request header when returned true. This allows for
// dynamic evaluation of allowed origins. Note if AllowCredentials is true, wildcard origins
// will be not have the 'access-control-allow-credentials' header set to 'true'.
//
// Optional. Default: nil
AllowOriginsFunc func(origin string) bool

// AllowOrigin defines a list of origins that may access the resource.
// AllowOrigin defines a comma separated list of origins that may access the resource.
//
// Optional. Default value "*"
AllowOrigins string
Expand All @@ -41,7 +43,8 @@ type Config struct {
// AllowCredentials indicates whether or not the response to the request
// can be exposed when the credentials flag is true. When used as part of
// a response to a preflight request, this indicates whether or not the
// actual request can be made using credentials.
// actual request can be made using credentials. Note: If true, AllowOrigins
// cannot be set to a wildcard ("*") to prevent security vulnerabilities.
//
// Optional. Default value false.
AllowCredentials bool
Expand Down Expand Up @@ -105,6 +108,26 @@ func New(config ...Config) fiber.Handler {
log.Warn("[CORS] Both 'AllowOrigins' and 'AllowOriginsFunc' have been defined.")
}

// Validate CORS credentials configuration
if cfg.AllowCredentials && cfg.AllowOrigins == "*" {
panic("[CORS] Insecure setup, 'AllowCredentials' is set to true, and 'AllowOrigins' is set to a wildcard.")
}

// Validate and normalize static AllowOrigins if not using AllowOriginsFunc
if cfg.AllowOriginsFunc == nil && cfg.AllowOrigins != "" && cfg.AllowOrigins != "*" {
validatedOrigins := []string{}
for _, origin := range strings.Split(cfg.AllowOrigins, ",") {
isValid, normalizedOrigin := normalizeOrigin(origin)
if isValid {
validatedOrigins = append(validatedOrigins, normalizedOrigin)
} else {
log.Warnf("[CORS] Invalid origin format in configuration: %s", origin)
panic("[CORS] Invalid origin provided in configuration")
}
}
cfg.AllowOrigins = strings.Join(validatedOrigins, ",")
}

// Convert string to slice
allowOrigins := strings.Split(strings.ReplaceAll(cfg.AllowOrigins, " ", ""), ",")

Expand All @@ -123,22 +146,18 @@ func New(config ...Config) fiber.Handler {
return c.Next()
}

// Get origin header
origin := c.Get(fiber.HeaderOrigin)
// Get originHeader header
originHeader := c.Get(fiber.HeaderOrigin)
allowOrigin := ""

// Check allowed origins
for _, o := range allowOrigins {
if o == "*" {
for _, origin := range allowOrigins {
if origin == "*" {
allowOrigin = "*"
break
}
if o == origin {
allowOrigin = o
break
}
if matchSubdomain(origin, o) {
allowOrigin = origin
if validateDomain(originHeader, origin) {
allowOrigin = originHeader
break
}
}
Expand All @@ -147,8 +166,8 @@ func New(config ...Config) fiber.Handler {
// handling the value in 'AllowOrigins' does
// not result in allowOrigin being set.
if allowOrigin == "" && cfg.AllowOriginsFunc != nil {
if cfg.AllowOriginsFunc(origin) {
allowOrigin = origin
if cfg.AllowOriginsFunc(originHeader) {
allowOrigin = originHeader
}
}

Expand All @@ -173,9 +192,17 @@ func New(config ...Config) fiber.Handler {
c.Set(fiber.HeaderAccessControlAllowOrigin, allowOrigin)
c.Set(fiber.HeaderAccessControlAllowMethods, allowMethods)

// Set Allow-Credentials if set to true
if cfg.AllowCredentials {
c.Set(fiber.HeaderAccessControlAllowCredentials, "true")
// When AllowCredentials is true, set the Access-Control-Allow-Origin to the specific origin instead of '*'
if allowOrigin != "*" && allowOrigin != "" {
c.Set(fiber.HeaderAccessControlAllowOrigin, allowOrigin)
c.Set(fiber.HeaderAccessControlAllowCredentials, "true")
} else if allowOrigin == "*" {
log.Warn("[CORS] 'AllowCredentials' is true, but 'AllowOrigins' cannot be set to '*'.")
}
} else {
// For non-credential requests, it's safe to set to '*' or specific origins
c.Set(fiber.HeaderAccessControlAllowOrigin, allowOrigin)
}

// Set Allow-Headers if not empty
Expand Down

0 comments on commit f0cd3b4

Please sign in to comment.