Re: Re: Making addslashes() multibyte aware
On Tue, Dec 17, 2013 at 5:54 AM, Adam Harvey <aharvey@php.net> wrote:
> On 16 December 2013 12:44, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
> > I don't think locale based MBCS support is optimum, but I'll add it to
> > addslashes() for now.
>
> I don't think basing behaviour on the locale is a great idea (as
> evidenced by the various issues with Turkish and Azeri over the
> years). Could we just add an explicit encoding parameter like
> htmlspecialchars()?
>
That's an option, but it requires a lot of work w/o mbstring being a
default "compiled" module.
>
> > Question is where should I start?
> > It's security issue for certain char encodings such as SJIS/BIG5.
>
> Is there a case other than database access (where we've been directing
> users to better APIs like PDO and mysqli for several years, at least)
> where this is likely to cause security issues?
When users are exporting PHP variables that are supposed to be parsed
as PHP (or PHP data, e.g. serialize()), addslashes()ed SJIS/BIG5/etc will
break string quoting with certain chars.
This can be used to execute attacker provided PHP code.
> > I'll fix php_addslashes(). Therefore, any functions that use it
> internally
> > are affected. e.g. var_export(), etc. Users are not affected as long as
> > they are using correct locale.
> >
> > Should I fix this from 5.3?
>
> This feels like it has the potential to be a really nasty implicit BC
> break. I don't think we'd want to change default behaviour on any
> stable branch, honestly.
I cannot object this argument.
There may be unexpected side effects.
However, users will not be affected as long as they are using correct
locale if locale system is not broken...
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Thread (7 messages)