Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:93142
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.220.172 as permitted sender)
MIME-Version: 1.0
Sender: yohgaki@gmail.com
In-Reply-To: <e9b34da1-a6c7-261e-1a83-9bf34d2ed8ac@gmail.com>
References: <CAGa2bXZytbD1H-Rg5eWfiPwEM0p47JSQH-GC-ZayDSLbmbsGVA@mail.gmail.com>
 <e9b34da1-a6c7-261e-1a83-9bf34d2ed8ac@gmail.com>
Date: Tue, 10 May 2016 19:57:23 +0900
Message-ID: <CAGa2bXZp5HavxNw6ZFk0RnOc2HvAtukLz2mnT8FC_UE8O9PNmw@mail.gmail.com>
To: Rowan Collins <rowan.collins@gmail.com>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] [RFC DRAFT] Automatic CSRF Protection
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

Hi Rowan,

On Tue, May 10, 2016 at 6:38 PM, Rowan Collins <rowan.collins@gmail.com> wrote:
> Yasuo Ohgaki wrote on 10/05/2016 04:24:
>>
>> Hi all,
>>
>> It's not nice to work on the same code (i.e. session module) for
>> multiple RFCs, but time is limited.
>>
>> I would like to hear from ideas/comments before I write patch for this.
>> https://wiki.php.net/rfc/automatic_csrf_protection
>
>
> I think rewriting every URL, and erroring if the token on a URL has expired,
> will not be useful for most people. What happens if I copy the URL of a page
> into an e-mail or Twitter post? As soon as anybody clicks that link, they're
> going to get an error raised; maybe it will recover by clearing their
> session, causing them to log out unnecessarily; maybe it will refuse to show
> the content claiming they're not authenticated. In the worst case, someone
> might take my URL and use the CSRF token against me - they have a time
> limit, but if the application author relied on this protection, the same
> token will be valid for any action on the site.

To protect all of URLs automatically, all URLs need to have token.
That's the reason why all URLs have token. The risk is the same as
Trans SID session management.

> As described, the feature seems to assume that all pages are potential CSRF
> targets, when even an authenticated user on a forum spends most of their
> time on URLs which retrieve data and have no side effects. As Stas pointed
> out, not all content is amenable to rewriting, either, which could lead to a
> false sense of security - dare I compare the infamous magic quotes? A good
> implementation of CSRF protection has to consider when to generate a token,
> when to check it, and what to do on failure - trying to submit a comment
> with an invalid CSRF token might re-display the comment form with pre-filled
> content, for instance - and this proposal doesn't seem to address that.
>
> I think this is the kind of feature that can only really be addressed by an
> application framework, which can have greater knowledge of when actions are
> being triggered, link tokens to specific actions, and so on.

Because of  likelihood of the vulnerability, it's better provide basic
infrastructure. IMO. It's possible to give more control to users.

 - Specify protected method GET/POST or both.
 - Add session_csrf_validate() for manual validation

Implementation is simple enough even with these addition. (Added to RFC)

I considered to protect whole application while I was writing this RFC
draft. Thank you for point this out!

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net