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

🐛 Using a non-standard session config leads to unexpected results #1040

Closed
hugmouse opened this issue Nov 27, 2020 · 1 comment
Closed

Comments

@hugmouse
Copy link

hugmouse commented Nov 27, 2020

Fiber version
Fiber v2.2.1

Issue description

If you use a non-standard config when initializing a session, it leads to the following consequences:

  • Set-cookie becomes invalid because the field with the cookie name becomes ""

Set-Cookie: cb98ba09-c8c4-4803-b8da-35e358174a0a; max-age=86400; path=/; SameSite=Lax

  • A new ID is created for each request (by using code snippet do some localhost:3000/get GET requests)
2020/11/27 19:41:23 ID: 759f465c-db2b-453c-9e6d-8de31a8f997f
2020/11/27 19:41:23 ID: 769f465c-db2b-453c-9e6d-8de31a8f997f
2020/11/27 19:41:24 ID: 779f465c-db2b-453c-9e6d-8de31a8f997f
2020/11/27 19:41:24 ID: 789f465c-db2b-453c-9e6d-8de31a8f997f
2020/11/27 19:41:25 ID: 799f465c-db2b-453c-9e6d-8de31a8f997f
2020/11/27 19:41:25 ID: 7a9f465c-db2b-453c-9e6d-8de31a8f997f
2020/11/27 19:41:25 ID: 7b9f465c-db2b-453c-9e6d-8de31a8f997f
2020/11/27 19:41:25 ID: 7c9f465c-db2b-453c-9e6d-8de31a8f997f
2020/11/27 19:41:25 ID: 7d9f465c-db2b-453c-9e6d-8de31a8f997f
2020/11/27 19:41:26 ID: 7e9f465c-db2b-453c-9e6d-8de31a8f997f
2020/11/27 19:41:26 ID: 7f9f465c-db2b-453c-9e6d-8de31a8f997f
2020/11/27 19:41:26 ID: 809f465c-db2b-453c-9e6d-8de31a8f997f

btw isn't the UUID being incremented is a vulnerability?


This can be avoided if you fill in a little more information in a non-standard config. For example:

store = session.New(session.Config{
	Expiration: 24 * time.Hour,
	CookieName: "session_id",
})

But I think that this behavior is wrong and if a user has set an attribute, the default values should be applied where the user has not changed the data initially.

For example config like this:

store = session.New(session.Config{
	Expiration:     24 * 7 * time.Hour,
})

Should be like this on Fiber's backend:

store = session.New(session.Config{
	Expiration:    24 * 7 * time.Hour,
	CookieName:    "session_id", // on 2.2.1 -- it's "" 
	KeyGenerator:  utils.UUID,   // on 2.2.1 -- not empty, but doing weird stuff
})

Code snippet

package main

import (
	"github.com/gofiber/fiber/v2"
	"github.com/gofiber/fiber/v2/middleware/session"
	"log"
	"time"
)

func main() {
	app := fiber.New()

	store := session.New(session.Config{Expiration: 24 * time.Hour})

	app.Post("/set", func(c *fiber.Ctx) error {
		// get session from storage
		sess, err := store.Get(c)
		if err != nil {
			panic(err)
		}
		// save session
		// Set key/value
		sess.Set("name", "john")

		log.Println("ID:", sess.ID())

		err = sess.Save()
		if err != nil {
			return err
		}
		return c.Send([]byte(sess.ID()))
	})

	app.Get("/get", func(c *fiber.Ctx) error {
		sess, err := store.Get(c)
		if err != nil {
			panic(err)
		}
		log.Println("ID:", sess.ID())
		return nil
	})

	err := app.Listen(":3000")
	if err != nil {
		panic(err)
	}
}

Log

* Preparing request to http://localhost:3000/set
* Current time is 2020-11-27T11:34:50.035Z
* Using libcurl/7.69.1 OpenSSL/1.1.1g zlib/1.2.11 brotli/1.0.7 libidn2/2.1.1 libssh2/1.9.0 nghttp2/1.41.0
* Using default HTTP version
* Disable timeout
* Enable automatic URL encoding
* Enable SSL validation
* Enable cookie sending with jar of 1 cookie
* Found bundle for host localhost: 0x7fe8b77bf030 [serially]
* Can not multiplex, even if we wanted to!
* Re-using existing connection! (#4) with host localhost
* Connected to localhost (127.0.0.1) port 3000 (#4)

> POST /set HTTP/1.1
> Host: localhost:3000
> User-Agent: insomnia/2020.4.2
> Cookie: session_id=0462255d-4a43-4314-a982-b1e4d23694e6
> Accept: */*
> Content-Length: 0

* Mark bundle as not supporting multiuse

< HTTP/1.1 200 OK
< Date: Fri, 27 Nov 2020 11:34:49 GMT
< Content-Length: 0
< Set-Cookie: cb98ba09-c8c4-4803-b8da-35e358174a0a; max-age=86400; path=/; SameSite=Lax


* Connection #4 to host localhost left intact
* Rejected cookie: Cookie failed to parse
* Saved 1 cookie
@Fenny
Copy link
Member

Fenny commented Nov 30, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants