Make WordPress Core

Opened 15 years ago

Closed 14 years ago

Last modified 14 years ago

#8689 closed defect (bug) (fixed)

WordPress must not use preg_replace with /e

Reported by: benbe's profile BenBE Owned by: ryan's profile ryan
Milestone: 2.8 Priority: high
Severity: major Version: 2.7
Component: Security Keywords: has-patch tested commit
Focuses: Cc:

Description

When a server runs the Suhosin Patch one option allows web administrators to enable certain security-related functionality like disabling remote URL inclusion or disabling certain functions like eval with much better granularity. One of this options you can choose from is to disable the /e modifier of the preg_replace command as this modifier allows for arbitary code to get executed.

If this option is enabled parts of WordPress stop working. So you either can patch all those locations by hand every time you need to update your WordPress blog or stop using WordPress or kindly ask the developers to cease using preg_replace with /e modifier and instead switch to using preg_replace_callback which in return provides you with much more flexibility.

Affected locations in WordPress 2.7 (DE version) are:

File wordpress\wp-admin\import\blogger.php
     553  		$post_content = preg_replace('|<(/?[A-Z]+)|e', "'<' . strtolower('$1')", $post_content);
     606  		$comment_content = preg_replace('|<(/?[A-Z]+)|e', "'<' . strtolower('$1')", $comment_content);
File wordpress\wp-admin\import\blogware.php
      92  			$post_content = preg_replace('|<(/?[A-Z]+)|e', "'<' . strtolower('$1')", $post_content);
     132  					$comment_content = preg_replace('|<(/?[A-Z]+)|e', "'<' . strtolower('$1')", $comment_content);
File wordpress\wp-admin\import\livejournal.php
      73  			$post_content = preg_replace('|<(/?[A-Z]+)|e', "'<' . strtolower('$1')", $post_content);
     109  					$comment_content = preg_replace('|<(/?[A-Z]+)|e', "'<' . strtolower('$1')", $comment_content);
File wordpress\wp-admin\import\rss.php
     106  			$post_content = preg_replace('|<(/?[A-Z]+)|e', "'<' . strtolower('$1')", $post_content);
File wordpress\wp-admin\import\wordpress.php
     384  		$post_excerpt = preg_replace('|<(/?[A-Z]+)|e', "'<' . strtolower('$1')", $post_excerpt);
     389  		$post_content = preg_replace('|<(/?[A-Z]+)|e', "'<' . strtolower('$1')", $post_content);
File wordpress\wp-content\plugins\ajaxd-wordpress\control\aWP-admin.php
      55  			$options = preg_replace('!s:(\d+):"(.*?)";!e', "'s:'.strlen('$2').':\"$2\";'", $options );
File wordpress\wp-includes\class-phpmailer.php
    1423          $encoded = preg_replace('/([\000-\011\013\014\016-\037\075\077\137\177-\377])/e',
File wordpress\wp-includes\formatting.php
    1151  		$subject = preg_replace('#\=([0-9a-f]{2})#ei', "chr(hexdec(strtolower('$1')))", $subject);
File wordpress\wp-includes\kses.php
     397  	return preg_replace('%((<!--.*?(-->|$))|(<[^>]*(>|$)|>))%e',
    1002  	$string = preg_replace('/&#([0-9]+);/e', 'chr("\\1")', $string);
    1003  	$string = preg_replace('/&#[Xx]([0-9A-Fa-f]+);/e', 'chr(hexdec("\\1"))', $string);
File wordpress\wp-includes\post-template.php
     226  		$output =	preg_replace('/\%u([0-9A-F]{4,4})/e',	"'&#'.base_convert('\\1',16,10).';'", $output);
File wordpress\wp-includes\js\tinymce\plugins\spellchecker\classes\GoogleSpell.php
     109  		$string = preg_replace('~&#x([0-9a-f]+);~ei', 'chr(hexdec("\\1"))', $string);
     110  		$string = preg_replace('~&#([0-9]+);~e', 'chr(\\1)', $string);

In order to quickly find the places where such a call is present, you can use the following regular expression:

/preg_replace\s*\(\s*'(.).+?\1[^']*?e[^']*'/

Attachments (3)

preg-replace.patch (7.9 KB) - added by beaulebens 15 years ago.
The attached patch fixes all the occurrences of preg_replace identified in this ticket that are part of the the WP core (doesn't cover external libraries).
pregreplaceeremove.twomore.patch (2.3 KB) - added by tbaboon 15 years ago.
pregreplaceeremove.twomore.v2.patch (2.5 KB) - added by tbaboon 15 years ago.

Download all attachments as: .zip

Change History (20)

#1 @jacobsantos
15 years ago

Two affected are external libraries, which needs to have the defect reported upstream.

  1. wordpress\wp-includes\js\tinymce\plugins\spellchecker\classes\GoogleSpell.php
  2. wordpress\wp-includes\class-phpmailer.php

One is a plugin, which again needs to be reported at the plugin trac.

The instances in kses, should already be fixed, but it can be looked at again.

The others do need to be corrected.

#2 @jacobsantos
15 years ago

  • Keywords Suhosin removed
  • Summary changed from preg_replace with /e forbidden with Suhosin patch to WordPress should not use preg_replace with /e

#3 @jacobsantos
15 years ago

  • Summary changed from WordPress should not use preg_replace with /e to WordPress must not use preg_replace with /e

#4 @westi
15 years ago

  • Cc westi added
  • Keywords needs-patch added

I thought we tried to banish preg_replaces with eval before...

Seems we missed some.

@beaulebens
15 years ago

The attached patch fixes all the occurrences of preg_replace identified in this ticket that are part of the the WP core (doesn't cover external libraries).

#5 @ryan
15 years ago

(In [10339]) Use preg_replace_callback instead of preg_replace with eval. Props beaulebens. see #8689

#6 @ryan
15 years ago

(In [10392]) Use preg_replace_callback instead of preg_replace with eval. Props beaulebens. see #8689 for 2.7

#7 @Denis-de-Bernardy
15 years ago

  • Milestone changed from 2.7.2 to 2.8

/e is still around in 2.8/trunk:

  • funky_javascript_fix()
  • PHPMailer::EncodeQP()

#8 @tbaboon
15 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

Here's a patch for the last two reported by Denis-de-Bernardy.

-- tbaboon

#9 @Denis-de-Bernardy
15 years ago

  • Keywords changed from has-patch, needs-testing to has-patch needs-testing

#10 @Denis-de-Bernardy
15 years ago

tiny suggestion: write the actual functions, or store the lambda function's name. else, using them repeatedly will create unneeded functions and gobble up memory.

#11 @tbaboon
15 years ago

  • Cc tbaboon added

OK, here's a second version of the patch.

As an aside, there are a bunch of other similar create_function() problems in the code base already, so you may want to open a separate bug for that (in case there isn't one already).

-- tbaboon

#12 @Denis-de-Bernardy
15 years ago

  • Keywords tested commit added; needs-testing removed

#13 @ryan
15 years ago

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

(In [11098]) Eliminate preg_replace with /e. Props tbaboon. fixes #8689

#14 @hakre
15 years ago

thanks!

#15 follow-up: @BenBE1987
14 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

WordPress 2.8.4 violates this again in /wp-admin/includes/post.php around line 6800 in function _fix_attachment_links().

#16 in reply to: ↑ 15 @westi
14 years ago

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

Replying to BenBE1987:

WordPress 2.8.4 violates this again in /wp-admin/includes/post.php around line 6800 in function _fix_attachment_links().

Please don't re-open tickets fixed in old releases but instead create a new ticket for the issue.

#17 @westi
14 years ago

I have created #10896 to track this issue - patches welcome.

Note: See TracTickets for help on using tickets.