PHP eval code sandbox break

Question

I noticed a heavily downvoted comment in here: http://php.net/manual/en/function.php-check-syntax.php

function eval_syntax($code)
{
    $braces = 0;
    $inString = 0;

    // We need to know if braces are correctly balanced.
    // This is not trivial due to variable interpolation
    // which occurs in heredoc, backticked and double quoted strings
    foreach (token_get_all('<?php ' . $code) as $token)
    {
        if (is_array($token))
        {
            switch ($token[0])
            {
            case T_CURLY_OPEN:
            case T_DOLLAR_OPEN_CURLY_BRACES:
            case T_START_HEREDOC: ++$inString; break;
            case T_END_HEREDOC:   --$inString; break;
            }
        }
        else if ($inString & 1)
        {
            switch ($token)
            {
            case '`':
            case '"': --$inString; break;
            }
        }
        else
        {
            switch ($token)
            {
            case '`':
            case '"': ++$inString; break;

            case '{': ++$braces; break;
            case '}':
                if ($inString) --$inString;
                else
                {
                    --$braces;
                    if ($braces < 0) return false;
                }

                break;
            }
        }
    }

    // If $braces is not zero, then we are sure that $code is broken.
    // We run it anyway in order to catch the error message and line number.

    // Else, if $braces are correctly balanced, then we can safely put
    // $code in a dead code sandbox to prevent its execution.
    // Note that without this sandbox, a function or class declaration inside
    // $code could throw a "Cannot redeclare" fatal error.

    echo "Braces: ".$braces."\r\n";
    $braces || $code = "if(0){{$code}\n}";

    if (false === eval($code)) {}
}

eval_syntax("file_put_contents('/home/yourname/Desktop/done.txt', 'OVERWRITTEN');");

I tried to bypass the code and lead to malicious user-input execution with eval, but I couldn't. I wonder why it got downvoted.

As you can see if curly brackets are not matching, it doesn't add the 'if(0){' . $code . '} and executes the user input as-is with mismatching curly brackets which will throw exception and won't really run.

If curly brackets are a match, it calls eval, but since its inside if {0} "sandbox", it doesn't do anything. How can someone bypass this?

I know eval is insecure, but I want to know what's the trick here. How can you bypass security of if (0) and braces check in the code above?

I tried to add // { so it will imbalance the curly brackets and it won't add the if (0) and call eval, but token_get_all ruins that.

Answer 1

I think this comment from the code you posted sums up my thoughts:

// We need to know if braces are correctly balanced. // This is not trivial due to variable interpolation // which occurs in heredoc, backticked and double quoted strings

Keep in mind that the result of improperly executing the code check is a remote code execution vulnerability. Working on non-trivial items is of course the norm for engineers, but if you are working on a task that is extremely difficult and a mistake leads to the most serious kind of security vulnerability - well, clearly that is a recipe for disaster. In fact it's worth pointing out that the comment you posted was the authors second attempt at this. His first attempt at contained at least one mistake that (presumably) was vulnerable to "sandbox escaping" and required him to start over. I'm certainly not going to trust the security of my application to a one off script written in a comment box on some random page on the web. Especially since (unless I'm missing something) this comment happens to be on a page describing a language built-in that does the exact same thing. I would trust the language built-in before some random guy on the internet.

Regardless of whether or not anyone has found an exploit yet, it doesn't change the fact that this is generally a bad idea in the first place. What if there are other escaping options that the author hasn't thought of? For instance, plenty of exploits out there have relied upon confusion over different encodings. It might be possible for an attacker to inject in a brace using an encoding different than the one in the file, but still supported by PHP, which will therefore not register as a brace in his checks but will be recognized as such by PHP, allowing an attacker to break through the brace balance check. Granted, that particular attack avenue may not actually be possible, but I'm mainly trying to emphasize the general point. There can be many hard-to-catch "gotchas" in situations like this, and the result of missing one of them in this code is a remote code execution vulnerability.

When faced with "Get it perfect or be extremely vulnerable" my first preference is "Just don't do it". That is followed up by a strong "don't trust a random comment on the internet", especially if there is an easily-accessible language built-in that does exactly the same thing but (presumably) securely.

Edit to add. It's worth making a comment about eval. You said "I know eval is insecure, but"... This is not strictly true. eval usage is generally frowned upon for a few reasons:

1. If nothing else it tends to have poor performance
2. If used improperly it certainly can create real security issues
3. It generally violates best programming practices

However, like any tool, it can have its time and place. So to be clear the issue isn't that eval is insecure. The question is whether or not eval is being used securely (and properly) here. For all the reasons outlined above I don't think that it is being properly used here, but that is different than a blanket "eval is insecure" statement.