Is this use of PHP eval() exploitable?

Question

During a code review, I have found something similar to this:

function foo() {

        $a = "a";
        $expression = '$_GET["$a"]';

        return eval('return ('.$expression.');');
}

$a = foo();
echo $a;

Don't mind the nonsense, the example is simplified and due to bad design decisions, the client "really" needs the "return eval" instruction.

My guess would be no, because $expression is not $_GET["$a"], but a string representing the name of the variable, so the argument to eval() is equivalent to a "return string".

But according to the PHP manual:

You should never use parentheses around your return variable when returning by reference, as this will not work. You can only return variables by reference, not the result of a statement. If you use return ($a); then you're not returning a variable, but the result of the expression ($a) (which is, of course, the value of $a).

Is there a way this could be exploited?

Answer 1

Since you just echo a GET variable, this is vulnerable to XSS. But that has nothing to do with your use of eval which I take is the main focus of the question. So let's look at that.

If you replace eval(...) with echo ..., you will note that no matter what you pass in, the output will be this:

return ($_GET["$a"]);

This is because you use ' and not ", so $expression does not end up conatining the value of the GET variable. So there is nothing controlled by the user in the expression you are evaluating - It's basically just a constant. That's not dangerous (given that the constant itself doesn't contain dangerous code), but it's not exactly useful either. You might as well just drop the eval.

But still... I would be nervous if I was responsible for this code. It sure looks shaky, and just using eval no matter why is a huge red flag. Plus, who knows what I might have missed in this answer.