# [PATCH] create_function/zend_eval_string aren't binary-safe

- Thread: https://externals.io/message/44134
- Original archive: https://news-web.php.net/php.internals/44134
- Started by Matt Wilmas on 2009-06-03T18:45:17+00:00
- Messages: 5
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #44134 — Matt Wilmas — 2009-06-03T18:45:17+00:00

URL: https://externals.io/message/44134#44134

Hi all,

I first noticed this with `preg_replace()`'s /e modifier (SO many other issues 
with that...), but it also happens with create_function() and a few other 
places where zend_eval_string() is used.  Other code evaluation in PHP is 
binary-safe, so it seems like these areas should be as well?  In case an 
example is needed:

$func = create_function('', "return strlen('Test\0string');");
echo $func();

Patches:
http://realplain.com/php/binary_eval_string.diff
http://realplain.com/php/binary_eval_string_5_3.diff

Can they be applied?  (Also a small optimization by eliminating `strlen()` 
usage.)  May want to verify the ext/interbase change.

I noticed there are several zend_eval_string() references in PECL sources 
[1], which would be a problem after adding a string length parameter...  How 
is that handled?  Should the updated function be eval_stringL instead and 
add macros for compatibility?

[1] http://lxr.php.net/ident?i=zend_eval_string


Thanks,
Matt

---

## #44135 — Andrei Zmievski — 2009-06-03T20:16:33+00:00

URL: https://externals.io/message/44134#44135 · In reply to: #44134

Matt Wilmas wrote:
> Hi all,
> 
> I first noticed this with `preg_replace()`'s /e modifier (SO many other 
> issues with that...), but it also happens with create_function() and a 
> few other places where zend_eval_string() is used.  Other code 
> evaluation in PHP is binary-safe, so it seems like these areas should be 
> as well?  In case an example is needed:
> 
> $func = create_function('', "return strlen('Test\0string');");
> echo $func();
> 
> Patches:
> http://realplain.com/php/binary_eval_string.diff
> http://realplain.com/php/binary_eval_string_5_3.diff
> 
> Can they be applied?  (Also a small optimization by eliminating `strlen()` 
> usage.)  May want to verify the ext/interbase change.

Yes, apply them (though I didn't verify the interbase patch).

> I noticed there are several zend_eval_string() references in PECL 
> sources [1], which would be a problem after adding a string length 
> parameter...  How is that handled?  Should the updated function be 
> eval_stringL instead and add macros for compatibility?

I'd prefer that.

-Andrei

---

## #44138 — johannes@schlueters.de — 2009-06-03T20:56:45+00:00

URL: https://externals.io/message/44134#44138 · In reply to: #44134

Hi,


> I noticed there are several zend_eval_string() references in PECL sources 
> [1], which would be a problem after adding a string length parameter...  How 
> is that handled?  Should the updated function be eval_stringL instead and 
> add macros for compatibility?

typical naming would be using eval_string_ex for the version with the
length, but didn't look into it and will be offline till tuesday. Bu if
others review and agree it is a good thing to change. (incl 5.3)

johannes

---

## #44145 — Matt Wilmas — 2009-06-04T02:55:38+00:00

URL: https://externals.io/message/44134#44145 · In reply to: #44135

Hi Andrei, all,

----- Original Message -----
From: "Andrei Zmievski"
Sent: Wednesday, June 03, 2009

> Matt Wilmas wrote:
> > […]
>
> Yes, apply them (though I didn't verify the interbase patch).

All right. :-)

> > […]
>
> I'd prefer that.

It seems the TSRMLS params would be an issue with macros (or am I not 
thinking right?), so I just used wrapper functions...  Patches are 
updated -- didn't check changes yet, but hopefully I didn't miss anything.

> -Andrei

Thanks,
Matt

---

## #44540 — Lukas Kahwe Smith — 2009-06-30T13:25:53+00:00

URL: https://externals.io/message/44134#44540 · In reply to: #44134

> Hi all,
>
> I first noticed this with `preg_replace()`'s /e modifier (SO many  
> other issues with that...), but it also happens with  
> create_function() and a few other places where zend_eval_string() is  
> used.  Other code evaluation in PHP is binary-safe, so it seems like  
> these areas should be as well?  In case an example is needed:
>
> $func = create_function('', "return strlen('Test\0string');");
> echo $func();
>
> Patches:
> http://realplain.com/php/binary_eval_string.diff
> http://realplain.com/php/binary_eval_string_5_3.diff
>
> Can they be applied?  (Also a small optimization by eliminating  
> `strlen()` usage.)  May want to verify the ext/interbase change.
>
> I noticed there are several zend_eval_string() references in PECL  
> sources [1], which would be a problem after adding a string length  
> parameter...  How is that handled?  Should the updated function be  
> eval_stringL instead and add macros for compatibility?
>
> [1] http://lxr.php.net/ident?i=zend_eval_string


should this be applied now as it hasnt been applied yet, afaik?

regards,
Lukas Kahwe Smith
mls@pooteeweet.org