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

Conflicting cookies shenanigans with SESSION_COOKIE_DOMAIN #5462

Open
azmeuk opened this issue Apr 9, 2024 · 7 comments · May be fixed by #5464
Open

Conflicting cookies shenanigans with SESSION_COOKIE_DOMAIN #5462

azmeuk opened this issue Apr 9, 2024 · 7 comments · May be fixed by #5464

Comments

@azmeuk
Copy link

azmeuk commented Apr 9, 2024

Hi,
I encountered a strange behavior of flask regarding cookies when the value of SESSION_COOKIE_DOMAIN is updated after some cookies have been set.
I observed this with Python 3.11, Flask 3.0.2 and Werkzeug 3.0.2 with both Firefox 124 and Chrome 123.
First of all, I could not reproduce the issue when serving on http://localhost:5000, so you might need to add 127.0.0.1 flask.localhost in your /etc/hosts to be able to reproduce this, and access the app from http://flask.localhost:5000.

  1. Put the following snippet in an app.py and run it with env FLASK_DEBUG=1 FLASK_APP=app flask run.
import flask
import base64
app = flask.Flask(__name__)
app.config["SECRET_KEY"] = "super secret"
app.config["SERVER_NAME"] = "flask.localhost:5000"
app.config["SESSION_COOKIE_DOMAIN"] = app.config["SERVER_NAME"]

@app.route("/", methods=("GET", "POST"))
def index():
    if flask.request.form:
        flask.session["value"] = flask.request.form["value"]

    session_cookies = [
        base64.b64decode(cookie.split(".")[0]).decode()
        for cookie in flask.request.cookies.getlist("session")
    ]
    return (
        '<form action="/" method="post"><input name="value"><input type="submit"></form"><br>\n'
        + f'value {flask.session.get("value")}<br>\n'
        + "cookies: " + " ".join(session_cookies)
    )
  1. Open http://flask.localhost:5000, clean the cookies if existing.
    flask.session is empty.
  2. Write foo in the form, validate.
    session["value"] contains foo.
  3. Reload the page
    session["value"] still contains foo.
    The firefox dev tools indicate that the cookie domain is .flask.localhost (with a leading dot).
  4. Comment the app.config["SESSION_COOKIE_DOMAIN"] = app.config["SERVER_NAME"] line
  5. Reload the page
    session["value"] still contains foo.
  6. Write bar in the form, validate.
    session["value"] still contains bar.
  7. Reload the page
    session["value"] contains foo, which is unexpected.
    A cookie has been set on step 6, and the firefox dev tools indicate that the cookie domain is flask.localhost (without a leading dot).
    However it seems the value is read from the step 2. cookie. that is still around.
    flask.request.cookies.getlist("session") contains two cookies, with the values foo and bar.

Those steps work also by starting with the SESSION_COOKIE_DOMAIN commented and then uncomment it at step 4.
The issue is not reproductible without the app.config["SERVER_NAME"] = "flask.localhost:5000" line.

The SESSION_COOKIE_DOMAIN documentation mentions that different values produce different cookies. However it does not mention that different cookies could conflict, neither that changing the value should be avoided.

Maybe there is some misconfiguration on my side (is it?) but nonetheless it feels strange that Flask could write in one session cookie (step 6.) and then read from another session cookie (step 7.).

Deleting the cookies solve the situation, however I can not expect the users of my production application to delete their cookies so the application is functionning again. And the curse is that some of my users have their cookies created with subdomains support, and some other have their cookies without subdomains support. So setting or deleting SESSION_COOKIE_DOMAIN will fix the issue for ones while provoking it for the other ones.

What do you think?

@davidism
Copy link
Member

davidism commented Apr 9, 2024

I am confident that we are currently doing the right thing with cookie domain. I spent a long time looking through the relevant current specs and browser behavior when reviewing that code last year.

Given the current way browsers handle cookies, it is less secure to set the domain property than to leave it unset. You're seeing the result of the two different behaviors here. When you set a domain, the cookie is valid for that and all subdomains. When you don't set a domain, the browser makes it valid only for the domain that requested it. When both cookies are set, the browser has to pick one to send first.

@davidism
Copy link
Member

davidism commented Apr 9, 2024

You can attempt to issue an few extra response.delete_cookie calls for each setting on some response, but beyond that we can't really affect what the browser stores and sends if you start sending it different overlapping things at different times.

@azmeuk
Copy link
Author

azmeuk commented Apr 9, 2024

When both cookies are set, the browser has to pick one to send first.

Does Flask has to pick the first one sent by the browser, or would it make sense for Flask to use the cookie with the most suitable domain, if that information is ever available?

You can attempt to issue an few extra response.delete_cookie calls for each setting on some response

response.delete_cookie("session", "flask.localhost") works, but response.delete_cookie("session", ".flask.localhost") does not, whatever the value of SESSION_COOKIE_DOMAIN.

@davidism
Copy link
Member

davidism commented Apr 9, 2024

The domain information is not present in the Cookie request header, it is only key=value.

A leading dot is irrelevant in modern browsers, it's equivalent to the same domain without the dot. So both those calls are the same. The other call would be delete_cookie without the domain at all.

@azmeuk
Copy link
Author

azmeuk commented Apr 9, 2024

I see, thank you for your insight.
In the end I could solve the situation simply by abandoning both cookies by changing the SESSION_COOKIE_NAME.

I think this would be worth mentioning in the documentation though. Would you be OK if I write a little caveat paragraph about this?

@davidism
Copy link
Member

davidism commented Apr 9, 2024

Sure, but where? It might be mentioned in the Werkzeug API docs already. Maybe in the security section for the Flask docs?

@azmeuk
Copy link
Author

azmeuk commented Apr 10, 2024

I can think of the Configuration Handling > SESSION_COOKIE_DOMAIN section, or in the Security > Set-Cookie Options section indeed.

I would expect to read this more on the configuration I think. As you want.

@azmeuk azmeuk linked a pull request Apr 15, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants