Page 1 of 1

Hack me! -- template security

Posted: Sat Jun 12, 2004 5:39 pm
by Heavy
Here is a simple class that can be extended for more functionality. It is responsible for making my templates secure for later being "included()".

I wrote a templating class during latest fall that did it all in one big fat ugly class. I am rewriting it now, with more and smaller classes. I stole the regex from my old version and immediately discovered a security hole :roll: . It was about the <script language="php"></script> style php tags (that I hate). I improved it so I at least can't break it myself.

So, the class goes like this:

Code: Select all

<?php
//Filename: class.tplsecurity.php
class tplsecurity
{
	var $strTemplate;
	function tplsecurity(& $strTemplate)
	{
		$this->strTemplate = & $strTemplate;
	}
	
	/**
	* @return void
	* @desc operates on $this->strTemplate and disables any PHP code in it.
	*/
	function stripPHP()
	{
		$this->strTemplate = preg_replace(
		'@(?i)'.
		'(<%(php)?)'. // small asp-style php tag
		'|'.
		'(<\?(?!xml\s))'. // small php tag, but leave xml tags untouched
		'|'.
		'(<script[^>]+((?i)language\s*=\s*("|'')php("|'')[^>]*>))'. //<script language="php"> php code </script>    [^<]*[^>]*(/script>)
		'@'
		,'[parse error]'
		,$this->strTemplate);
	}
	
}

?>
And here is a test script:

Code: Select all

<?php
	include("class.tplsecurity.php");
	header("Content-type: text/plain");
	
$TestString=
<<<INPUT
<?xml version="1.0" encoding="iso-8859-1"?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<title>Login</title>
<meta name="GENERATOR" content="Quanta Plus" />
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1" />
</head>
<body>
<script language='php' >echo "boo boo!"; </script>

It should not just say "boo boo!" above. 
If it does, there is a hole in the regex.
Also, there should not be any problem with the xml tag on the first line.
If allow short open tags is on, then the xml tag will generate a parse error in eval() below. Inserting a space before xml makes the regex catch the "<?" and disable it, so then you'll be fine.

</body>
</html>
INPUT;


$objSeq = new tplsecurity( & $TestString);
$objSeq->stripPHP();
eval("?>" . $objSeq->strTemplate);

?>
Anyone seeing any more holes?
Gotta be secure...

Posted: Mon Jun 14, 2004 7:48 am
by Weirdan

Code: Select all

<script language=php>echo "boo boo!"; </script>

Posted: Mon Jun 14, 2004 7:52 am
by Weirdan
why include() the file if you don't want php in it to be executed?

Posted: Mon Jun 14, 2004 9:13 am
by Heavy
Weirdan wrote:

Code: Select all

<script language=php>echo "boo boo!"; </script>
My dearest thank you!
I didn't think of that.
Here comes the hero:

Code: Select all

<?php
class tplsecurity
{
	var $strTemplate;
	function tplsecurity(& $strTemplate)
	{
		$this->strTemplate = & $strTemplate;
	}
	
	/**
	* @return void
	* @desc operates on $this->strTemplate and disables any PHP code in it.
	*/
	function stripPHP()
	{
		$this->strTemplate = preg_replace(
		'@(?i)'.
		'(<%(php)?)'. // small asp-style php tag
		'|'.
		'(<\?(?!xml\s))'. // small php tag, but leave xml tags untouched
		'|'.
		'(<script[^>]+(language\s*=\s*("|'')?php("|'')?[^>]*>))'. //<script language="php"> php code </script>
		'@'
		,'[parse error]'
		,$this->strTemplate);
	}
	
}
?>
Weirdan wrote:why include() the file if you don't want php in it to be executed?
This function makes sure the one who wrote the template does not succeed putting dangerous php in it.

The template syntax allows variable substitution with the style {varName}, which by the template engine will be replaced for <?php echo $varName;?>

I need to include it, because the template engine PUTS php in it for the sake of automation. But before the template engine does that, I want to make sure there's no php in it already, because any initial php is considered malicious.

Process is:
1 disable php
2 translate template file and put php into it then cache it to disk.
3 include the cached version of the file into the calling script, to output the presentation layer.

Get it now?

Posted: Mon Jun 14, 2004 9:16 am
by Weirdan
Ah, makes much more sense now ;)

Posted: Mon Jun 14, 2004 9:17 am
by Heavy
Man, this gives me a good feeling!
One should never be afraid to post code that might be not as good as one wants it to be!
Thank you for your input!

Posted: Mon Jun 14, 2004 11:19 am
by Weirdan
just keep in mind that you should always replace possibly dangerous strings with something, not just with empty string. Otherwise something like:

Code: Select all

<<?? echo "BOOOOO!"; ?>
would be possible.

Posted: Tue Jun 15, 2004 3:11 am
by Heavy
Yes.
I've been there... here, it is replaced with [parse error]
One paranoic attempt would be to loop that check until it doesn't change anything anymore.