Skip to content

Comments

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

Open
martin762 wants to merge 1 commit intomasterfrom
martin762-patch-1
Open

Add cookie prefix '-__Secure-' to cookies to help prevent cookie smuggling (issue18608) #1
martin762 wants to merge 1 commit intomasterfrom
martin762-patch-1

Conversation

@martin762
Copy link
Owner

The PR 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

Please describe your pull request.
The raison d'aitre for this PR is contained in my issue # 18608.
That description referred to the file 'libraries/libraries/classes/config.php'. I have been guided to the path 'phpmyadmin/src/Config.php' for the PR. Thank you WilliamDes.

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' : ‘’ );

Descriptive commit message:
The PR 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).

(A further enhancement might be by a check of the path and substituting '__Host-' if the path is '/'. This is not part of the PR.)

As per guidance I have directed this PR to QA_5_2

Signed-off-by: martin762 martin762green@btinternet

The PR 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).
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.

1 participant