Re: [VOTE] Multbye char handling - Remove vulnerability related to multibyte short and long term

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