wp_kses_normalize_entities regular expression does not use callback

[darkdragon]

In wp_kses_normalize_entities(), the second preg_replace uses the 'e' or eval instead of using the preg_replace_callback() function that has been in PHP since 4.0.5.

Recommendation:

Change

$string = preg_replace('/&#0*([0-9]{1,5});/e', 'wp_kses_normalize_entities2("\\1")', $string);

To:

$string = preg_replace_callback('/&#0*([0-9]{1,5});/', 'wp_kses_normalize_entities2', $string);

[darkdragon]

wp_kses_bad_protocol_once() is also the same.

[lloydbudd]

Replying to darkdragon:

In wp_kses_normalize_entities(), the second preg_replace uses the 'e' or eval instead of using the preg_replace_callback() function that has been in PHP since 4.0.5.

What is the benefit?

[westi]

The main issue with e is that you are giving user supplied data to php to evaluate - therefore theoretically you could have a security issue if you are not careful.

This is why using a callback is better.

[darkdragon]

I was wrong about wp_kses_bad_protocol_once(), since from what I've read on php.net on preg_replace_callback() it does not allow for adding parameters and the replacement in that needs to have a parameter passed to the callback function. Which is not possible.

I pointed it out since using 'e' replacement parameter has bitten phpBB quite a few times and is generally seen as being a security risk. I'm unsure if that stands here, since I'm not a security expert.

Preventing something could go a long way however, since the fix is relatively trivial and should not break anything.

I'm unsure how much support you have for the Kses library.

[ryan]

Switching to preg_replace_callback() is good. I just did this in another place for [7056]. Anyone want to patch this up?

[ryan]

(In [7106]) Use preg_replace_callback instead of 'e' modifier. see #5644

[ryan]

(In [7107]) Use preg_replace_callback instead of 'e' modifier. see #5644

[ryan]

Fixed wp_kses_bad_protocol_once() with a global hack on allowed_protocols. I had it working by var_export()ing allowed_protocols into the callback string, but var_export can be messed up if it is run within a containing ob_start.

[darkdragon]

That is cool. Thanks.

I totally forgot about this ticket.