Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🚀 [Feature]: stop dangerously bypassing the wildcard exception (CORS) #2338

Closed
3 tasks done
jub0bs opened this issue Feb 17, 2023 · 5 comments · Fixed by #2339
Closed
3 tasks done

🚀 [Feature]: stop dangerously bypassing the wildcard exception (CORS) #2338

jub0bs opened this issue Feb 17, 2023 · 5 comments · Fixed by #2339

Comments

@jub0bs
Copy link

jub0bs commented Feb 17, 2023

Feature Description

Fiber's CORS middleware actively bypasses the so-called wildcard exception: if developers configure their CORS middleware to allow credentials and specify the wildcard as an allowed origin, the resulting middleware unconditionally reflects the value of the request's Origin header in the Access-Control-Allow-Origin response header.

This is insecure insofar as it exposes users to cross-origin attacks that can be mounted from any origin.

For information, a similar issue was reported to (and subsequently fixed by) other Web frameworks/libraries:

Additional Context (optional)

Steps to reproduce

  1. Run mkdir wildcardcraziness && cd $_.
  2. Save the program below to main.go.
  3. Run go mod init whatever && go mod tidy.
  4. Run go run main.go.
  5. Run curl -sD - -o /dev/null -H "Origin: https://attacker.org" localhost:8081/hello.

Expected behaviour

Perhaps the following:

curl -sD - -o /dev/null \
  -H "Origin: https://attacker.org" \
  localhost:8081/hello
HTTP/1.1 200 OK
Access-Control-Allow-Credentials: true
Access-Control-Allow-Origin: *
-snip-

Ideally, though, the resulting middleware should not be built at all, since it is dysfunctional. More about this in my latest blog post.

Actual behaviour

curl -sD - -o /dev/null \
  -H "Origin: https://attacker.org" \
  localhost:8081/hello
HTTP/1.1 200 OK
Access-Control-Allow-Credentials: true
Access-Control-Allow-Origin: https://attacker.org
-snip-

Note the unconditional reflection of the request's Origin header value (https://attacker.org) in the Access-Control-Allow-Origin response header.

Code Snippet

package main

import (
	"log"

	"github.com/gofiber/fiber/v2"
	"github.com/gofiber/fiber/v2/middleware/cors"
)

func main() {
	app := fiber.New()
	app.Use(cors.New(cors.Config{
		AllowOrigins:     "*",
		AllowCredentials: true,
	}))
	app.Get("/hello", hello)
	if err := app.Listen(":8081"); err != nil {
		log.Fatal(err)
	}
}

func hello(c *fiber.Ctx) error {
	return c.SendString("Hello, World!")
}

Checklist:

  • I agree to follow Fiber's Code of Conduct.
  • I have checked for existing issues that describe my suggestion prior to opening this one.
  • I understand that improperly formatted feature requests may be closed without explanation.
@welcome
Copy link

welcome bot commented Feb 17, 2023

Thanks for opening your first issue here! 🎉 Be sure to follow the issue template! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

@ReneWerner87
Copy link
Member

Thx for the report

@ryand67
Copy link
Contributor

ryand67 commented Feb 18, 2023

I'd love to take a shot at this!

@leonklingele
Copy link
Member

leonklingele commented Feb 18, 2023

Using a default config of AllowOrigins: "*" looks like something we definitely should try to avoid — imo even if we break v2 compatibility.

@li-jin-gou
Copy link
Contributor

I'd love to take a shot at this!

thanks and already assign you

@ryand67 ryand67 mentioned this issue Feb 20, 2023
12 tasks
ryand67 added a commit to ryand67/fiber that referenced this issue Feb 20, 2023
@ReneWerner87 ReneWerner87 linked a pull request Feb 20, 2023 that will close this issue
12 tasks
ReneWerner87 pushed a commit that referenced this issue Feb 20, 2023
🐛- fix cors * behavior #2338
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants