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

Add cookie prefix '-__Secure-' to cookies to help prevent cookie smuggling (issue18608) #19141

Open
wants to merge 1 commit into
base: QA_5_2
Choose a base branch
from

Conversation

martin762
Copy link

The aim of this PR is to help prevent cookie smuggling. It will modify the 'GetCookieName' functtion by hard coding the prefix ' __Secure-' to each cookie name when 'isHttps()' is true, Apparently the prefix will be recognised and enforced by most browsers (ignored by the older ones).

Description

The raison d'aitre for this PR is contained in the issue # 18608.

This PR will replace the line 1026 of the file 'Config.php', part of the GetCookieName function :
return $cookieName . ( $this->isHttps ? '_https' : '' );

with the amended line:
return ( $this->isHttps() ? '__Secure-' : '’ ) . $cookieName . ( $this->isHttps() ? '_https' : ‘’ );

The amendment is on the QA_5_2 branch and path libraries/Classes

Signed-off-by: martin762 <[martin762green@btinternet.com>

…refix ' __Secure-' to each cookie name when 'isHttps()' is true, Apparently the prefix will be recognised and enforced by most browsers (ignored by the older ones).Update Config.php

The raison d'aitre for this PR is contained in my issue # 18608.


This PR will replace the line 953, part of the GetCookieName function :
return $cookieName . ( $this->isHttps ? '_https' : '' );

with the amended line:
return ( $this->isHttps() ? '__Secure-' : '’ ) . $cookieName . ( $this->isHttps() ? '_https' : ‘’ );
@martin762 martin762 marked this pull request as ready for review May 1, 2024 16:17
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 this pull request may close these issues.

None yet

2 participants