-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Make session cookies httponly by default #13720
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
base: master
Are you sure you want to change the base?
Conversation
Thanks. You would need to have a look at the failed tests :
|
Regarding hardening The benefit of the change is questionable. How does it strengthen security? In my opinion, it just gives the information to the attackers on which session ID is occupied or not. With this option disabled, attackers don't get this information as every session ID is valid for them. Regarding |
Hi @jorgsowa,
Is php.net wrong then? |
I can't find the proper discussion about this topic right now. One of them you have here: https://externals.io/message/94484#94583 I don't have a strong opinion and also I don't know the implementation details to be sure about the proper solution. I just wanted to inform you that changing this value has some trade-offs and this should be again discussed on the internals list. Regardless, documentation and the default setting clearly mismatch, which should be improved either way. Thank you for raising this topic. |
Thank you so much, I really appreciate it. It's my first time digging in this repo, so sorry about me not knowing the internal discussion protocols. |
I think of this scenario: A (attacker) was able inject unescape javascript (xss) into a post which B (user) can see, js can to see all |
@DaWe35 could you send your thoughts on the internals mailing list? I'm not a core developer and you will get more feedback there. I think the topic you raised is important and should be addressed by more experienced devs. You can find a guide on how to enroll on the mailing list |
Hi @DaWe35 , do you need help with writing RFC? It would be good to be ready with this change on the next PHP version. |
yes, thank you, this stupid email list kills me |
I'm glad you are continuing the topic. The proper email list you need to send the message to is internals@lists.php.net |
@DaWe35, since quite some time you need to be subscribed to the mailing list to send messages (a spam reduction measure). On https://www.php.net/mailing-lists.php you find a form for online subscription (that may work, or not; I've never tried it), and also instructions for manual subscription (what worked for me a couple of weeks ago). So please, try again! |
@cmb69 I was subscribed and all my emails bounced. I don't like when projects make it a hustle to report a bug, I'm trying to help, but I'm not willing to spend more time on this. It seems the PHP team doesn't want me to contact them. This should have been fixed years ago anyway |
Maybe @derickr or @jimwins have an idea what went wrong.
Highly unlikely. :) |
@DaWe35, the message states that you wouldn't be subscribed to the mailing list. Did you actually use the same email address ( |
@cmb69 yes I mean I got the denial email about not being able to subscribe |
Ah, I see. Apparently, the form at mailing-list.php is still broken, since it sends to the wrong recipient ( But you can also subscribe via email; just send a mail to PS: php/web-master#27 should fix this issue. |
I've merged the ML PR. Will take up to an hour to make it to the published site. |
Story
My website I wrote 9 years ago was hacked recently. It was quite tricky to exploit (I mean it took 9 years for someone to discover the bug), but I made a mistake and my website was vulnerable to XSS.
I'm a self-taught programmer and PHP was the first language I learnt. I think 9 years ago I did a pretty good job eliminating XSS and other injections, but I never expected anyone to go this deep in a hobby website and actually compose a working exploit.
Importance of default values
With the injected code, the attacker was able to steal session cookies. I'm trying hard to make session cookies httponly on all my websites, but somehow this one was not set (probably because of the age of the site). I believe this attack wouldn't have any consequences if httponly were 1 by default. I hope my input helps to make the web better, and I'm sorry if my knowledge lacks. I really believe defaults should be safe (I tried to read all the related discussions eg. #7913)
httponly
I believe it is uncommon that a website needs javascript access for a session cookie*, therefore making httponly=1 by default would not break many websites. I also believe it would make a lot of website much safer, since there are a tons of websites with the insecure default. Changing this default value is a net positive in my opinion.
use_strict_mode
According to php.net:
My only question is that why it's not default? I can hardly imagine changing this variable resulting in any damage or broken code. If it is advised to be 1, I think it should be 1 by default.
secure??In this PR, I did not include any changes related to secure, but I'd be happy to make more defaults secure.
I think changing secure to 1 would be amazing, but I also understand that this can break things (for example development mostly happens on localhost without SSL, and some websites still run in flexible SSL mode on Cloudflare). I'm not sure what is the best approach to this, I think we should still try to move forward with secure defaults, but displaying a helpful error message is essential so at least dev envs can be fixed in no time, when this config variable is missing in the local dev env.
Why not all cookies, why only sessions?
In this PR, I only changed defaults related to session cookies
I'm not sure if it's a good idea to set all cookies to httponly=1, but as I said, the safer is better, so I can support that. It will cause inconvenience and break things in some situations, therefore I'm not changing any setcookie() or setrawcookie() defaults now.