Invalid free of CG(interned_empty_string)

[manuelm]

On failure php_escape_html_entities returns STR_EMPTY_ALLOC which is an
alias of CG(interned_empty_string) if interned strings are enabled.
Make sure we don't free this.

Fixes #68996

[weltling]

@manualm, could you please provide a reproduce case for this? Or even better, add a phpt?

Thanks.

[manuelm]

The bug report includes one-liners to trigger the bug. Every invalid free has its own line. However there's no phpt because you can't easily check the content of CG(interned_empty_string) from php.

[weltling]

@manuelm, yeah, but phpt is what we usually do when referencing some erroneous function call. Also that ensures the functionality is correct in future.

Why I ask for this again is because i don't reproduce invalid reads you mention in the ticket, valgrind 3.8.1 . Is there something special needed, maybe some ini options? I'd still ask for adding a phpt, so one can run it with and without your patch to tell the diff.

Thanks.

[manuelm]

For the first-listed invalid free you need to enable the html_errors ini flag. The others should be traceable out of the box.

[manuelm]

@weltling USE_ZEND_ALLOC=0 valgrind php -dhtml_errors=1 -r 'fopen("\xfc\x63", "r");' works for me. Same for the other calls.
Edit: "Works" as in shows the invalid free.

Do you still want me to provide a phpt-file with the calls included?

[laruence]

this should also exists in 5.5, but not master

[manuelm]

Sure about this? imho interned_empty_string has been introduced in PHP-5.6

see https://github.com/php/php-src/blob/PHP-5.5/Zend/zend.h#L676

[weltling]

@manuelm ok, behavior confirmed. Despite it's only reproducible with valgrind, I'd say a phpt would still be useful because "make test" can be done with valgrind enabled and then it'd be catchable. The patch should go in anyway, but it would be really nice to have some phpt as well.

@laruence looks like i cant repro it in 5.5 and master, but 5.6 ... probably it's 5.6 only

Checking a bit more yet.

Thanks.

[laruence]

@manuelm you are right. I remembered it wrongly

[manuelm]

Tests added. I've split them among the various extensions. Hope that's ok.

[weltling]

@manuelm thanks for the tests, checking all together to merge tomorrow. So if you're adding some more stuff till then, it's fine.

Thanks.