Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:72564
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.214.173 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAGa2bXaq+S8hhPW4Mx1KTWjhXk=OL+J5hHWoS-t0Q-5hJOxBow@mail.gmail.com>
References: <CAGa2bXaq+S8hhPW4Mx1KTWjhXk=OL+J5hHWoS-t0Q-5hJOxBow@mail.gmail.com>
Date: Thu, 13 Feb 2014 22:14:11 +0100
Message-ID: <CAF+90c-OQfVX+X34ONwYOhmbvw6JQ_QYJ-k-ksE__B-sTD7jTA@mail.gmail.com>
To: Yasuo Ohgaki <yohgaki@ohgaki.net>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=089e01182660bbbe9504f2502b63
Subject: Re: [PHP-DEV] [VOTE] Multbye char handling - Remove vulnerability
 related to multibyte short and long term
From: nikita.ppv@gmail.com (Nikita Popov)

--089e01182660bbbe9504f2502b63
Content-Type: text/plain; charset=ISO-8859-1

On Mon, Feb 10, 2014 at 4:56 AM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:

> Hi all,
>
> Current PHP has security issue that attacker may execute arbitrarily script
> via encoding based attack. These 2 RFCs are for short and long term
> resolution for this issue.
>
>
>
> Short term: Multibyte Char Handling
> https://wiki.php.net/rfc/multibyte_char_handling
> Add functions required to resolve security issues. CVE-2014-1239
>
>
> Long term: Alternative implementation of mbstring using ICU
> https://wiki.php.net/rfc/altmbstring
> We need multibyte feature as default. However, current mbstring has license
> issues. Resolve license issues by alternative mbstring in the future.
> Introduce mbstring-ng as EXPERIMENTAL module for further development,
> testing, feedback from users.
>
> VOTE: 2014/02/10 - 2014/02/17
>
>
>
> Please note that these RFCs are independent from whether PHP is going to
> support Unicode in core or not. It's about how PHP provides required
> features. Even when PHP supports UTF-8 as string encoding, these multibyte
> features are required. Otherwise, encoding parameter/property is mandatory
> for core multibyte support. We need to support existing applications long
> term also.
>
> Since it is security related issue, please provide/propose alternative
> resolution if anyone feels it is not the way to go. There must be feasible
> resolution.
>
> Thank you for voting and/or alternative proposal!
> Even though alternative is better to proposed during discussion period,
> better alternative is welcomed at anytime.
>
> Regards,
>
> P.S. We are better to have comment field in RFC. It is not constructive
> just voting "No" without reason/alternative. Alternatively, we may have
> rule
> to post mail here explains the reason why.
>

I voted no on both RFCs, for about the same reasons as Joe. Voting on
mbstring-ng at this point makes about zero sense, given that the code is
nowhere even near being ready for merging into PHP.

Regarding the other RFC for the addition of new mb_* function, I'd like to
start off by saying that I disagree with your premise that this is a
security vulnerability that needs to be fixed quickly and across all
supported versions. As far as I can see the issue is somebody using
addslashes() in an inappropriate context - this is a vulnerability in the
application, not PHP. This is a lot like saying that we have an RCE
vulnerability in eval() because someone had the genius idea of putting
eval($_GET['str']) in his or her code.

As this constitutes no critical or non-critical vulnerability in PHP, any
function additions should only happen in PHP 5.6 and not PHP 5.4+.

Apart from this, the RFC is very low on technical detail. It doesn't have a
patch, which is not a problem *per-se*, however it doesn't have any
implementational details whatsoever. While I guess that the addslashes part
is rather straightforward, I'm sure that addcslashes, escapeshellarg and
var_export are less trivial.

An example of what is unclear: The addclashes functions has a charlist
parameter. Currently chars with ASCII <32, >126 are encoded in octal - what
will happen here with multibyte chars? Will you encode the unicode code
point in octal? Are there even systems that support that? And what about
the range feature? Will this cover unicode code point ranges now? Etc.

This RFC probably has merit, but right now there is not enough detail to
make an informed decision.

Thanks,
Nikita

--089e01182660bbbe9504f2502b63--