Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:96458
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain ohgaki.net designates 180.42.98.130 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAGa2bXb0CKP6=eZZk2atR5XGtR8JmQ3FG756wn0J7kkPGYFyZg@mail.gmail.com>
References: <CAGa2bXb0CKP6=eZZk2atR5XGtR8JmQ3FG756wn0J7kkPGYFyZg@mail.gmail.com>
Date: Wed, 19 Oct 2016 10:00:29 +0900
Message-ID: <CAGa2bXYShMMU964gtq4VZfkBssJftJX-k2j+hRh6_YAyxm9mUw@mail.gmail.com>
To: "internals@lists.php.net" <internals@lists.php.net>, Davey Shafik <davey@php.net>
Cc: Xinchen Hui <laruence@php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: header() removes all header of the same name.
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

Hi all,

On Tue, Oct 18, 2016 at 4:31 PM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
> I understand why header() is made to remove all headers of the same
> name. This is needed in some cases, but it does not work well for some
> cases.
>
> We need to decide what to do with
> https://bugs.php.net/bug.php?id=72997
>
> There is 2 issues.
>   - header() removes all headers of the same name including 'Set-Cookie'
>   - header() ignores replace flag. (This one is easy to fix)
>
> Since header() enables 'replace flag' by default, it removes all
> 'Set-Cookie' headers sent previously by default. It can easily disturb
> security related cookies to work. i.e. Session ID cookie, Auto Login
> cookie. This bug would be very hard to find for normal users, too.
>
> Restoring older behavior (Removing only one header) cannot be a
> resolution because it can still disturb security related cookies.
>
> Possible resolutions:
>
>  - Prohibit 'Set-Cookie' for header() and force users to use setcookie()
>  - Mitigate by disabling replace flag by default. (This is not a good idea, IMO)
>
> Both resolution requires BC, but this is better to be fixed ASAP.
>
> Non-BC resolution could be:
>   - "Ask users to use setcookie() always for 'Set-Cookie'".
>
> I would like to prohibit 'Set-Cookie' by header() because it may
> remove session ID cookie as well as auto login cookie, etc. If we
> leave released version as it is now, I would like to prohibit
> 'Set-Cookie' by header() in PHP 7.1.
>
> Problem with this may be that user cannot modify 'Set-Cookie' header
> line as user want.
>
> $ php -r 'setcookie("REMEMBERME=value; expires=Sat, 03-Sep-2020
> 05:38:43 GMT; path=/; domain=aaa");'
> PHP Warning:  Cookie names cannot contain any of the following '=,;
> \t\r\n\013\014' in Command line code on line 1
>
>
> Comments?

An idea for session ID protection.

Following code results in lost session always.

<?php
session_start();
session_regenerate_id();
header('Set-Cookie: something');
?>

header() function removes all header of the same name, e.g.
Set-Cookie, Expires, etc, by sapi_remove_header(). This could be very
hard to find the cause.


This risk can be removed w/o much BC. Only BC is when user is
intentionally trying to delete session ID cookie manually. This would
be very rare. We can add code that excludes session ID cookie in
sapi_remove_header().
http://lxr.php.net/xref/PHP-MASTER/main/SAPI.c#593

To do that, we can search string like following
Set-Cookie: PHPSESSID=xxxxxxx

The only issue is we need session global, i.e. PS(session_name), at
least. It's not nice to have dependency from SAPI.c to session, but it
protects session ID from removed by users by mistake.

Any comments?

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net