Make WordPress Core

Opened 7 years ago

Closed 6 years ago

Last modified 5 years ago

#9602 closed defect (bug) (fixed)

eval problem in classes.php and rewrite.php

Reported by: obsidiandh Owned by: westi
Milestone: 2.8.5 Priority: low
Severity: minor Version: 2.7.1
Component: Permalinks Keywords: has-patch tested
Focuses: Cc:



One of the moderators on the support forums said I should raise a ticket for this.

wp-includes/classes.php and wp-includes/rewrite.php both use the eval function.

This function is disabled by suhosin (and sometimes in php.ini) for security reasons.

I've written a patch for the files which under my limited testing seems to have no ill effects.

I expect you can find a cleaner way to do this but here is the patch (see attached files).

Hopefully this will help you get wordpress installed on more locked down systems.



Attachments (3)

classes.php.patch (434 bytes) - added by obsidiandh 7 years ago.
classes.php patch
rewrite.php.patch (431 bytes) - added by obsidiandh 7 years ago.
rewrite.php patch
9602.patch (2.9 KB) - added by hakre 7 years ago.
Corrected Patch, Substition established with preg_replace_callback and a helper class.

Download all attachments as: .zip

Change History (28)

7 years ago

classes.php patch

7 years ago

rewrite.php patch

#1 @Denis-de-Bernardy
7 years ago

  • Component changed from General to Security
  • Owner changed from anonymous to ryan

#2 @Denis-de-Bernardy
7 years ago

  • Keywords has-patch added
  • Milestone changed from Unassigned to 2.8

#3 @Denis-de-Bernardy
7 years ago

  • Component changed from Security to Permalinks
  • Keywords needs-patch added; has-patch removed
  • Priority changed from normal to low
  • Severity changed from normal to minor

can't get the patch to work.

#4 follow-up: @hakre
7 years ago

could not handle the .patch format as well. just made a patch removing the in my opinion senseless eval calls against trunk but i must admit i am a little puzzled:

was: eval("\$query = \"" . addslashes($query) . "\";");
patch: $query = addslashes($query)

i do not understand why this eval was used there. as far as i can see it must be by mistake.

#5 @hakre
7 years ago

  • Keywords has-patch 2nd-opinion added; needs-patch removed

after clearification might be a good idea to adopt in the inline comment there.

#6 in reply to: ↑ 4 @Denis-de-Bernardy
7 years ago

  • Keywords dev-feedback added; 2nd-opinion removed

Replying to hakre:

i do not understand why this eval was used there. as far as i can see it must be by mistake.

same here. it looks truly weird. the original patch didn't seem to make much sense either.

Maybe Ryan knows why it was there.

#7 @DD32
7 years ago

eval is needed in that situtation (with harke's patch). However, The original idea to use something else.. Does have the side effect that its slower..

It converts the rewrite rule from "?page_id=$matches[0]&$matches[2]=$matches[3]" to "?page_id=2&test=you", ie. by evaluating the rule directly.

#8 @Denis-de-Bernardy
7 years ago

  • Keywords needs-patch added; has-patch dev-feedback removed

ah, ok. things makes sense now.

needed to create an attachment with verbose rules to see it work with is_local_attachment().

#9 @hakre
7 years ago

my fault, did oversee that $query contains code then. will try to convert the original patch to get further.

7 years ago

Corrected Patch, Substition established with preg_replace_callback and a helper class.

#10 @hakre
7 years ago

  • Keywords has-patch added; needs-patch removed

i took a look into the previous patches but the code looked to complex to me. i decided to go the route with preg_replace_callback encapsulated into an own object that can carry the $matches substitution data into the callback.

#11 @Denis-de-Bernardy
7 years ago

Very neat. To be very honest, I'd suggest we keep the (faster) eval() call, even if it breaks a crummy host or two. Safe mode is going to die soon enough. :-)

#12 @hakre
7 years ago

-1 for eval.

eval is evil. really, that makes stuff hard to debug and potentially unsecure. and while writing the patch and thinking about the substitutions it came into my mind that there is a design issue within this complex that should be (in the future) taken care of. Therefore I would put the finger in there and drop the use of eval.

the regexp callback should be really fast and the instance of the class has reasonable impact. have you run a speed comparison?

#13 @Denis-de-Bernardy
7 years ago

  • Keywords tested dev-feedback added

#15 @ryan
7 years ago

  • Milestone changed from 2.8 to 2.9

#16 @Denis-de-Bernardy
6 years ago

dev feedback welcome before the patch becomes stale

#17 @westi
6 years ago

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

Testing and reviewing this at the moment.

#18 @Hans Spath
6 years ago

I think eval() should be avoided at all cost.

I haven't checked if it's possible and it doesn't look like it, but if a site visitor somehow manages to modify or control parts of $query (not $matches), you have a serious problem (addslashes() won't protect you):


$userstr = '&blabla={$x[die(join(null,array_map(chr(99).chr(104).'
. 'chr(114),array(72,105,32,87,111,114,100,112,114,101,115,115,44,'
. '32,101,118,97,108,32,105,115,32,101,118,105,108,46,10))))]}';

$query = '?page_id=$matches[0]&$matches[2]=$matches[3]' . $userstr;
$matches = array('a', 'b', 'c', 'd');

eval("\$query = \"" . addslashes($query) . "\";");
echo "Hello world!";
echo "\$query is '$query'\n";


Hi Wordpress, eval is evil.

#19 @westi
6 years ago

(In [11853]) Replace eval usage in request processing with new WP_MatchesMapRegex() class usage. See #9602 props hakre.

#20 @westi
6 years ago

I've been testing this for a while now and it looks ok to me.

Putting into trunk for now to see how everyone else gets on.

Leaving this open for now for feedback.

#21 @westi
6 years ago

  • Keywords dev-feedback removed
  • Milestone changed from 2.9 to 2.8.5

Not had any issues / complaints with this so going to merge into the 2.8 branch as well.

#22 @westi
6 years ago

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

(In [11891]) Replace eval usage in request processing with new WP_MatchesMapRegex() class usage. Fixes #9602 for 2.8 branch props hakre.

#23 @Derevko
6 years ago

  • Cc giuseppe@… added

#24 @scribu
5 years ago

Note the scope resolution operator, which will produce a parser error in PHP4:


It looks like WP hasn't been compatible with PHP4 since 2.8.5 :)

#25 @scribu
5 years ago

Rats, it seems static calls are available in PHP4 too: http://www.php.net/manual/ro/keyword.paamayim-nekudotayim.php

Note: See TracTickets for help on using tickets.