WordPress.org

Make WordPress Core

Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#5644 closed defect (bug) (fixed)

wp_kses_normalize_entities regular expression does not use callback

Reported by: darkdragon Owned by: ryan
Milestone: 2.5 Priority: normal
Severity: normal Version:
Component: Security Keywords: kses
Focuses: Cc:
PR Number:

Description

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);

Change History (11)

#1 @darkdragon
12 years ago

  • Keywords kses added

#2 @darkdragon
12 years ago

wp_kses_bad_protocol_once() is also the same.

#3 in reply to: ↑ description @lloydbudd
12 years ago

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?

#4 @westi
12 years ago

  • Owner changed from anonymous to westi
  • Status changed from new to assigned

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.

#5 @darkdragon
12 years ago

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.

#6 @ryan
12 years ago

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

#7 @ryan
12 years ago

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

#8 @ryan
12 years ago

  • Owner changed from westi to ryan
  • Status changed from assigned to new

#9 @ryan
12 years ago

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

#10 @ryan
12 years ago

  • Resolution set to fixed
  • Status changed from new to closed

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.

#11 @darkdragon
12 years ago

  • Milestone changed from 2.6 to 2.5

That is cool. Thanks.

I totally forgot about this ticket.

Note: See TracTickets for help on using tickets.