Using PHP as template language

Edit: Great points all around, dedicated templating language is obviously the way to go. Thanks!

I wrote up this quick class to do templating via PHP -- I was wondering if this is easily exploitable if I were ever to open up templating to users (not the immediate plan, but thinking down the road).

class Template {

private $allowed_metho开发者_如何学编程ds = array(
    'if', 
    'switch', 
    'foreach',
    'for', 
    'while'
);

private function secure_code($template_code) {
    $php_section_pattern = '/\<\?(.*?)\?\>/';
    $php_method_pattern = '/([a-zA-Z0-9_]+)[\s]*\(/';
    preg_match_all($php_section_pattern, $template_code, $matches);
    foreach (array_unique($matches[1]) as $index => $code_chunk) {
        preg_match_all($php_method_pattern, $code_chunk, $sub_matches);
        $code_allowed = true;
        foreach ($sub_matches[1] as $method_name) {
            if (!in_array($method_name, $this->allowed_methods)) {
                $code_allowed = false;
                break;
            }
        }
        if (!$code_allowed) {
            $template_code = str_replace($matches[0][$index], '', $template_code);
        }
    }
    return $template_code;      
}

public function render($template_code, $params) {
    extract($params);
    ob_start();
    eval('?>'.$this->secure_code($template_code).'<?php ');
    $result = ob_get_contents();
    ob_end_clean();
    return $result;     
}

}

Example usage:

$template_code = '<?= $title ?><? foreach ($photos as $photo): ?><img src="<?= $photo ?>"><? endforeach ?>';
$params = array('title' => 'My Title', 'photos' => array('img1.jpg', 'img2.jpg'));
$template = new Template;
echo $template->render($template_code, $params);

The idea here is that I'd store the templates (PHP code) in the database, and then run it through the class which uses regular expressions to only allow permitted methods (if, for, etc.). Anyone see an obvious way to exploit this and run arbitrary PHP? If so, I'll probably go the more standard route of a templating language such as Smarty...

Sure..

$template_code = '<?= `rm -rf *`; ?>';

Edit:

Can't think of anything else right away. But you should know your scope is compromised IF render is ever called more than once on the same Template instance.

For example, if you render('<?php $this->allowed_methods[] = "eval"; ?>') .. then that instance of Template will have eval as an acceptable function for the next render .. ;)

This isn't a good idea. Even if you fix the immediate security holes, there will undoubtedly be others that you missed. I'd say if you really want to give users this ability, use an actual templating language such as Smarty, or write your own. PHP is great as a templating language for internal use, but not as an open one that all your users can use. There are just so many ways this can get exploited that even if you could catch them all, you'd do more work than writing a real templating engine.

you can use { and } with variables and run any function.

$template_code = '<?php $f = "phpinfo"; ${"f"}(); ?>';

Also, because you are just running the code, it would have access to all variables that the function render can access. including calling global to modify variables from pretty much anywhere or super globals such as $_SESSION (one such option might be session variables holding login information, and using javascript to post through ajax to another site).

$a = "hello";
$template_code = '<?php global $a; $a = "test"; ?>';
$params = array('title' => 'My Title', 'photos' => array('img1.jpg', 'img2.jpg'));
$template = new Template;
echo $template->render($template_code, $params);
echo $a;

another one, abusing the allowed functions by making a variable with the same name as an allowed function and the value of any function name.

$template_code = '<?php $if="phpinfo"; $if(); ?>';

I am wrong in thinking something like this would work?

<?php
/* ?> trick your parser by using a comment */
// do whatever unfiltered

If you really want to do something like this, use the tokenizer to parse the source. I don't recommend it though. In fact, I discourage it!

If you have users contributing content, and these users aren't totally trustworthy, I'd recommend allowlisting instead of denylisting markup.

For example, consider the Markdown formatting allowed by Stack Overflow. It supports a very short list of formatting options, and everything else is considered to be literal text. Most users are happier with a simple interface instead of an interface that says "Write any code you want! But be careful not to break the app!"

My rule of thumb is: allow users to enter data and content; never allow users to enter code.