Ouch. This is ringing the "gaping security hole" alarm... I hope that user input isn't part of the injected code in any way... indeed eval() must die.
Yeah, vBulletin "depends" on it to support hooks, but php's eval() function is truly evil.
However, at least the eval()'d code in question comes from a database, not from any user-supplied input, which as you say, would be *seriously* bad.
Duct tape to the rescue :-)
No kidding. Check this out:
// References to "vbam" need to check that it actually is an object before invoking methods on it, but they don't
// The basic type information would be nice for an instanceof check instead of this
if (strpos($hook, 'shareads')!==false) {
$hook = str_replace(
'$output = $vbam->',
'if (is_object($vbam)) $output = $vbam->',
$hook
);
}
eval($hook);
How nasty is that? :lol:
Luckily the offending string in $hook is about 20 lines of fairly basic code after I got it logged, with two method calls on $vbam, neither of which did any checks to see if $vbam even contains a value, let alone whatever object type it was expecting. This is a temporary fix until I can find out where in the database the actual code lives and fix it properly.
PS. I did get the 500 error once maybe an hour ago. Also the site seems to run a bit slow but only intermittently.
There are other potential sources of 500 errors, but I've not seen one caused by this particular problem since the duct tape above.
