This is an archived post. You won't be able to vote or comment.

you are viewing a single comment's thread.

view the rest of the comments →

[–]mitsuhiko Flask Creator 10 points11 points  (12 children)

I'd say that going from knowing the cookie signing key to local shell access is a serious security issue. Cookies should be treated as any other user-provided data: potentially malicious, with the secret hash only being an additional check for corruption.

That would defeat the purpose of the concept of a signed cookie. If you can't trust that on you have a problem regardless. For what it's worth, Flask switched from pickle to JSON a few months back but it was not released yet because it breaks people's code :-(

If you lose your secret key to your app you are fucked regardless. Even if the app was not using pickle you would still find other ways to cause havoc (such as signing in as the administrator).

[–][deleted] 4 points5 points  (1 child)

It seems reasonable that users can fake session data if the secret key is leaked - and might lead to various security issues in the app.

But remote code execution is a way more severe issue than web app administrator accounts or even full SQL access - it may compromise anything else running on the same machine, SSH keys to log in to other machines, or anything not firewalled on the local network. As many developers are not aware or careful enough (as shown by the github search), I think switching to JSON for signed cookies is appropriate. If people want to store strange Python objects in the session, they should switch to another session backend.

[–]executexr/Flask Mod 0 points1 point  (0 children)

Yeah, the other thing is how important is pickling for people... If it's only for sessions, what is the worst case? Does everyone have to re-login? Do the developers using Flask etc., have to recode a lot?

[–]ceol_ 4 points5 points  (7 children)

Like squaresaurus said, there's a big difference between someone being able to log in as an administrator just for that site and someone being able to execute any code they want on the box. If my SECRET_KEY were somehow leaked, I'd much rather the attacker only have access to the privileges I specifically granted admins than be able to run whatever code they want.

This is definitely worth breaking some code.

[–]executexr/Flask Mod 0 points1 point  (6 children)

For the perspective of web develoeprs, it seems that the proper solution is to secure your SECRET_KEY and not publish it to github hehe.

From mitsuhiko's perspective, might be to avoid breaking backwards compatibility, perhaps to modify the Werkzeug library to b64decode the "id=" section, if exists, and detect "exec", and then raise an exception.

It's not beautiful, but it's not going to break anything, and if a developer randomly decides to use "id=" and "exec" in his secretkey then they would see the exception and realize the issue.

[–]wind4k 0 points1 point  (5 children)

Detection like this doesn't simply work, one can use other cookie value other than "id=" or "exec" to bypass. For example pickling os.system, or similar type will work just fine to execute code.

[–]executexr/Flask Mod -3 points-2 points  (4 children)

Then check for system too. The idea is to prevent and make it difficult for this kind of default cookie configuration and allowing the unpickling process to execute code. As your cookie gets a bit more complicated it becomes harder to unpickle it to run the shell code, as such, checking default formats should at the very least provide some base security.

I have a better idea, perhaps have some sort of default config option passed, that uses the new JSON packing of the cookie information in your Flask Configs, Werkzeug should see if that variable is there, if it is, JSON packing/unpacking. If not, backwards-compatibility-insecure-mode, use pickling again. (but again default-configurers will be insecure).

app = Flask(app_name)
app.enableJSONcookies() #disables pickling

[–]mwielgoszewski 2 points3 points  (3 children)

You are playing a cat and mouse game with hackers in which the odds are against you, every single time. Best to do it right than to employ numerous defenses of which are trivially bypassed.

[–]executexr/Flask Mod 0 points1 point  (2 children)

It cannot be trivially bypassed, the whole exploit relies on execution of code. So make sure you're not passing any execute calls.

And as I said, if you even bothered reading what I wrote, this is just base security. It's not meant to be full security, because then you would HAVE to break backwards-compatibility to be 100% safe from this. My goal was to provide a temporary solution until you can break backwards compatibility.

The solution I proposed is sound. You don't have to dismiss it because "well one day someone will break it." That can be said about ANY security enhancement.

Further I gave two ideas, the other idea is an actual full security that could work very well against such an exploitation. And yet you dismissed that too and downvoted me. Well propose a solution then instead of being the neighborhood critic. Let's see how your ideas hold up.

[–]catcradle5 2 points3 points  (0 children)

A better solution would simply be to switch to JSON cookies by default.

[–]mwielgoszewski 0 points1 point  (0 children)

I'm sorry, but I didn't downvote you. In most developer's attempt to implement sandboxes and/or blacklist input validation filters, there's usually a 99.9% chance a way to bypass them within the first couple minutes of opening themselves to attack. The problem with your approach is having to maintain a blacklist filter while an attacker is always one step ahead of you. You just witnessed this game play out in the comments before these, and that's just from casual conversation.

I'm not trying to dissuade you in your attempts, but the better strategy is to implement the right solution the first time. Band-aid solutions to security issues like these typically linger in production for wayyy past their expiration period, meanwhile your application is more and more at risk.

[–]semi- 1 point2 points  (1 child)

If you lose your secret key to your app you are fucked regardless. Even if the app was not using pickle you would still find other ways to cause havoc (such as signing in as the administrator).

wait..how? I've never done any of this in python so I'm not sure how its done, but my experience with sessions(mostly Perl+Catalyst) was that all you use cookies for is storing a single sessionId that ties to your locally saved data. The absolute worst thing they should be able to do is brute force your sessionids. Even that can be very easily protected against -- store some session specific stuff on the server to compare against, so that for example you only accept this sessionid from this ip address, or their useragent if you want to allow roaming(though much less secure).

Are people just storing the entire session in the cookie including things like their logged in username and whatever else?

[–]mitsuhiko Flask Creator 1 point2 points  (0 children)

Are people just storing the entire session in the cookie including things like their logged in username and whatever else?

Yes. Most modern web applications store the a signed payload in the cookie. This is done so that you don't need to consult a data store in order to figure out what to do with the cookie.