By Island Usurper on

This is just a question meant to satisfy my curiosity. The documentation for drupal_eval() says,

"Using this wrapper also ensures that the PHP code which is evaluated can not overwrite any variables in the calling code, unlike a regular eval() call."

I want to see if anyone can explain why this is so. I suppose it's because of the output buffering, but I don't see any indication that that's right.

⋅ Categories: Drupal 5.x

Comments

dman’s picture

dman commented

I can't explain,

// Risky variable:
global $user;
print "<pre>".print_r($user,1)."</pre>";

// Possible exploit:
$subscript = 'global $user; $user->name = "superman" ; print "Changed User!"; ';

$result = drupal_eval( "<"."?php  ". $subscript . "  ?".">");
print $result;

print "<pre>".print_r($user,1)."</pre>";
// exploited!

stdClass Object
(
    [name] => myname
    ... 
)
Changed User!
stdClass Object
(
    [name] => superman
    ... 
)

Doesn't look any "safer" to me! I certainly can overwrite if I want.

But it does prevent accidental overwrites of the calling context.

$var = "contextual caller value";
print "Function value is $var <br/>";

// User script that may accidentally 
// re-use a var name from the calling function
$subscript = '$var = "handmade value"; ';

$result = drupal_eval( "<"."?php  ". $subscript . "  ?".">");

print "Function value is still $var <br/>";

eval($subscript );

print "Function value is now $var <br/>";

Function value is contextual caller value
Function value is still contextual caller value
Function value is now handmade value

.dan.
How to troubleshoot Drupal | http://www.coders.co.nz/

.dan. is the New Zealand Drupal Developer working on Government Web Standards

yonailo’s picture

yonailo
He/him
commented

You are right, it's not safe if you use "global" modifier but you have to explicitely use it beforehand.

I think the documentation should state this fact, that if you want to overwrite a variable you can
do it declaring it with "global". I see it as a feature more than a bug. It simply should be fixed
in the documentation (imho).