Jump to content
Sign in to follow this  
NotionCommotion

Please review template parser

Recommended Posts

I am sure this is a terrible idea and it was just for fun, but I would still appreciate a review of gaping security threats.  Thank you

 

<?php
require_once('parser.php');
$parser=new parser();
$results=$parser->makeTemplate('template.html');
echo ($results).'<hr>';


require_once('template_child.php');
$template=new template_child(array('color'=>'red','dog'=>'ONE','name'=>'Burt'));
$template->displayTemplate();

template.html

<div>
  <p>bla bla bla</p>EOD
  <p>Once there was a {{ color }} {{ animal }}.</p>
  <p>It's name was {{ name }}.</p>
  <p>The End!</p>
  <p>'</p>
  <p>/'</p>
  <p>'"</p>
  <p>{{ $this->bomb() }}</p>
  <p>{$this->bomb()}</p>
  <p>{\$this->bomb()}</p>
  <p>{\\$this->bomb()}</p>
  <p>{\\\$this->bomb()}</p>
</div>
<?php echo('I am a bad guy!');?>

parser.php

<?php


class parser{
    public function makeTemplate($template){
        if($template = file_get_contents($template) ){
            //Change following line to only add backslash if dollar sign currently has zero or and even number of preceeding backslashes
            $template = str_replace(['\\','$'], ['', '\$'], $template);
            $deliminator=$this->getDeliminator($template);
            $template = preg_replace('/{{ (\w+) }}/', '{$this->getValue("$1")}', $template);
            $template_class = <<<EOD
<?php
require_once('template.php');
class template_child extends template
{
    public function displayTemplate()
    {
        echo
        <<< $deliminator
        $template;
$deliminator;
    }
}
?>
EOD;
            $results=file_put_contents("template_child.php",$template_class);
            return $results?'Success':'Failed to store template';
        }
        else {return 'missing file';}
    }


    private function getDeliminator($template,$deliminator='EOD'){
        if(strpos($template, $deliminator)===FALSE){
            return $deliminator;
        }
        else {return $this->getDeliminator($template,$deliminator.rand(0,9));}
    }


}

template.php

<?php
class template
{
    protected $data=array();


    public function __construct(array $data=null)
    {
        if($data){$this->data=$data;}
    }


    protected function getValue($name)
    {
        return (isset($this->data[$name])?$this->data[$name]:null);
    }
    public function bomb()
    {
        exit('kaboom!');
    }
}

 

Share this post


Link to post
Share on other sites

Zero views after around 12 hours?  It is not that bad.  (EDIT.  That is odd.  Shows up as zero views, I add a post, and it changes to 62 views?) Also, it is only around 50 lines of code and not too much.  By the way, template.html should be considered suspect and data (i.e. array('color'=>'red','dog'=>'ONE','name'=>'Burt') ) should be considered safe.  Thanks

Edited by NotionCommotion

Share this post


Link to post
Share on other sites

Maybe not "review" is the right word.  I am not looking for a detailed review, just whether the approach has gaping security liabilities.  Any comments would be much appreciated.  Thank you

Share this post


Link to post
Share on other sites

Perhaps, you're not getting an answer because no one understands your question.  Is there something not working?  Do you have unexpected output?  

If you're just looking for security holes and suggestions and critiques to your system,then this belongs in Application Design.

 

Share this post


Link to post
Share on other sites

Though still, there is no answer to a critique.  A person needs to know the environment of the application, the usage, the context, the purpose, and so on in order to tell you what is bad about it.

 

If I had any suggestion at all it would be to not return strings with arbitrary values.  True or false is very powerful in programming, much more so than something like "Failed to store template".  Because then, you have to check later to see if that function returned "Failed to store template" and not "Failed to store templat" (note the missing e)

 

All that you're going to get out of your "question" is a bunch of "Why are you doing this that way?  You should do it this way {It's better practice})" yada yada.

Share this post


Link to post
Share on other sites

Hi Zane,

 

Thank you for your response and thank you for moving my post to Application Design.  In hindsight, I fully agree my original post was cryptic at best, and I should have given more context.  Please let me start over.

 

I wish to allow partially trusted users to easily create their own HTML templates using the "back end" application.  They would be allowed to upload a CSS file, images, and a HTML file which will define the basic structure.  My application would then take their HTML template file and convert it to a PHP file.

 

When viewing their sites front end, my application would generate the HTML using the newly created PHP template, and populating it with various information or HTML blocks of code.

 

In regards to being "partially" trusted, they are trusted not to implement XSS exploits, but I do not wish to give them access to the server other than described above.  A template engine such as Twig is a possibility, however, I don't need (or want) all the features of Twig in the template.

 

So, given the script described in my original post, is there anything a user could put in template.html which could give them access to the server or execute a PHP function?

 

Thank you

 

Share this post


Link to post
Share on other sites

I'm still not sure why you do not stick with an existing product, like Twig. All of the hassle is already done, and it's already been tested by the masses for security holes and such.

 

Plus people don't have to learn yet another templating syntax.

Edited by scootstah
  • Like 1

Share this post


Link to post
Share on other sites

Hi Scootstah,

 

I agree that Twig is great, but don't want most of the functionality; only replacing placeholders with different content.

 

Also, I am just curious whether there are any mistakes in my approach.  Please take a look and comment.

 

Thank you

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
Sign in to follow this  

×

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.