php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #78929 plus signs in cookie values are converted to spaces
Submitted: 2019-12-08 18:25 UTC Modified: 2020-01-28 14:30 UTC
Votes:1
Avg. Score:4.0 ± 0.0
Reproduced:1 of 1 (100.0%)
Same Version:1 (100.0%)
Same OS:1 (100.0%)
From: kachalin dot alexey at gmail dot com Assigned: cmb (profile)
Status: Closed Package: URL related
PHP Version: Irrelevant OS: Irrelevant
Private report: No CVE-ID: None
 [2019-12-08 18:25 UTC] kachalin dot alexey at gmail dot com
Description:
------------
While cookie parsing PHP violate RFC standards and change valid character + "plus" x2B into invalid character space x20

How to check 
1. Set cookie by php function setrawcookie(***)
2. Make sure it set correctly in browser
3. Make request with cookie and check that $_COOKIE[ COOKIE_NAME ] have cookie which plus sign was changed into space.

Valid characters for value       defined in RFC6265.4.1.1
Valid characters for name(token) defined in RFC6265.4.1.1. -> RFC2616.2.2

http://www.faqs.org/rfcs/rfc6265.html
http://www.faqs.org/rfcs/rfc2616.html

This applicable only for a value, because PHP documentation put restriction to use letters and numbers only for a cookie name.
I haven't found any restriction on value and believe value should use character defined by RFC.

Test script:
---------------
<?php

/* Run twice 
 * At first time to set the cookie.
 * At second time to validate the cookie value and check an error message.
 */
$cookieName  = 'RFC6265';
$cookieValue = '#$%&\'()*+-./0123456789<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^_`abcdefghijklmnopqrstuvwxyz{|}~!';

if(empty($_COOKIE[ $cookieName ]))// Set cookie for first time.
{
  setrawcookie($cookieName, $cookieValue);
}
else// Compare received cookie value with set value. Should be same. Echo on error.
{
  for($i = strlen($cookieValue)-1; $i > -1; --$i)
  {
    if(!isset($_COOKIE[ $cookieName ][ $i ]))
        {echo "\n<br>Cookie value symbol is lost:".     $cookieValue[ $i ];}
    elseif($cookieValue[ $i ] != $_COOKIE[ $cookieName ][ $i ] )
        {echo "\n<br>Cookie value symbol is different:".$cookieValue[ $i ];}
  }
}

Expected result:
----------------
Empty output. No error message on success execution.   

Actual result:
--------------
If error happened message will showed with wrong or lost characters.

Patches

Add a Patch

Pull Requests

Pull requests:

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2019-12-08 21:09 UTC] kachalin dot alexey at gmail dot com
The following pull request has been associated:

Patch Name: Fix #78929:  Fix a cookie parsing value. Switch to a php_raw_url_decode()
On GitHub:  https://github.com/php/php-src/pull/4989
Patch:      https://github.com/php/php-src/pull/4989.patch
 [2019-12-10 11:07 UTC] cmb@php.net
-Status: Open +Status: Verified
 [2019-12-10 11:07 UTC] cmb@php.net
Ugh, indeed, plus signs in cookie values should be left untouched.

However, the oldest PHP version which still has active support is
PHP 7.3, and I have doubts that we should fix this issue for that
version (not even sure about PHP 7.4) for BC reasons.
 [2019-12-10 14:06 UTC] kachalin dot alexey at gmail dot com
While making a good decision, please consider:

1. It's affected only for cookie set by setrawcookie() with "plus sign" inside value. The setcookie() properly encodes a "plus sign".

2. If cookie is set by external system, it's pretty hard to figure out that interaction is failed because of cookie parsing.
For example: Cookie WXYZ[]^+_'abcde looks same as WXYZ[]^ _`abcde

3. PHP 7.4 released 3 weeks ago. Most probably it will have long live for several years. Some developers can write software that already need a "plus sign" fix.
 [2019-12-12 13:13 UTC] cmb@php.net
-Summary: Cookie value parsing. Valid character changed into invalid. RFC6265 RFC2616 +Summary: plus signs in cookie values are converted to spaces -Assigned To: +Assigned To: cmb
 [2019-12-12 13:13 UTC] cmb@php.net
You're reasoning makes sense, so I agree that the bugfix should
target PHP 7.4
 [2019-12-12 13:23 UTC] cmb@php.net
Automatic comment on behalf of kachalin.alexey@gmail.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=79376ab209f61be03bbf8c1b6177c18261767da8
Log: Fix #78929: plus signs in cookie values are converted to spaces
 [2019-12-12 13:23 UTC] cmb@php.net
-Status: Verified +Status: Closed
 [2020-01-28 14:23 UTC] steve at vtsv dot ca
While this change fixes the case when using setrawcookie() with data containing a plus sign, it breaks the case when using setcookie() with data containing a space, as setcookie() will url encode the data, turning the space into a plus. The plus no longer gets converted back to a space, and so any validation will fail.
 [2020-01-28 14:30 UTC] cmb@php.net
Steve, see bug #79174.
 [2020-06-03 23:51 UTC] jaroslavas dot karmazinas at outlook dot com
The following pull request has been associated:

Patch Name: protect master branches except for the pecl repos against force pushes
On GitHub:  https://github.com/php/karma/pull/4
Patch:      https://github.com/php/karma/pull/4.patch
 
PHP Copyright © 2001-2020 The PHP Group
All rights reserved.
Last updated: Thu Oct 22 19:01:23 2020 UTC