Smarty incompatible in S9Y 1.7?

Found a bug? Tell us!!
garvinhicking
Core Developer
Posts: 30022
Joined: Tue Sep 16, 2003 9:45 pm
Location: Cologne, Germany
Contact:

Re: Smarty incompatible in S9Y 1.7?

Post by garvinhicking »

Hi!

I disagree. I know s9y installations where people give FTP access to the /templates/ directory and only *.tpl files, and for those people the security is meant, so that they cannot execute PHP code.

So the security definitely has a point for this; of course, once you have access to plugins PHP code, the security is useless.

Regards,
Garvin
# Garvin Hicking (s9y Developer)
# Did I help you? Consider making me happy: http://wishes.garv.in/
# or use my PayPal account "paypal {at} supergarv (dot) de"
# My "other" hobby: http://flickr.garv.in/
Timbalu
Regular
Posts: 4598
Joined: Sun May 02, 2004 3:04 pm

Re: Smarty incompatible in S9Y 1.7?

Post by Timbalu »

Hmmm, Garvin.
Yes there may be some out there who give ftp access and to *.tpls only, apart from having Knowledge about the problematic if not, but I expect this will be far less than 1% of all Serendipity installations. To keep an overhead for these few, is highly doubtful in my opinion.

It would make sense to have an option for this in the backend, which only enables Smarty-Security, if really needed.
Regards,
Ian

Serendipity Styx Edition and additional_plugins @ https://ophian.github.io/ @ https://github.com/ophian
garvinhicking
Core Developer
Posts: 30022
Joined: Tue Sep 16, 2003 9:45 pm
Location: Cologne, Germany
Contact:

Re: Smarty incompatible in S9Y 1.7?

Post by garvinhicking »

Hi!

But it's a feature of s9y, and I see no reason to kill that off... It worked in Smarty2, and from what I figure it's currently also possible with Smarty3, or am I misinterpreting this thread?

Regards,
Garvin
# Garvin Hicking (s9y Developer)
# Did I help you? Consider making me happy: http://wishes.garv.in/
# or use my PayPal account "paypal {at} supergarv (dot) de"
# My "other" hobby: http://flickr.garv.in/
blog.brockha.us
Regular
Posts: 695
Joined: Tue Jul 03, 2007 3:34 am
Location: Berlin, Germany
Contact:

Re: Smarty incompatible in S9Y 1.7?

Post by blog.brockha.us »

First 1.6 Blog reported, the oembed was working with

Code: Select all

$serendipity['smarty']->security_settings[@INCLUDE_ANY] = true;
/*
if (version_compare($serendipity['version'], '1.7-alpha1')>=0) {
    $serendipity['smarty']->disableSecurity();
}
else {
    $serendipity['smarty']->security = false;
}
*/
So: My actual oembed.tpl doesn't need switched off Smarty security.
- Grischa Brockhaus - http://blog.brockha.us
- Want to make me happy? http://wishes.brockha.us/
Timbalu
Regular
Posts: 4598
Joined: Sun May 02, 2004 3:04 pm

Re: Smarty incompatible in S9Y 1.7?

Post by Timbalu »

garvinhicking wrote:But it's a feature of s9y, and I see no reason to kill that off... It worked in Smarty2, and from what I figure it's currently also possible with Smarty3, or am I misinterpreting this thread?
No, its working, thats not the point. We can say, 'we had it and keep it', but my argues where just because we are using a more or less called 'security' feature, which is only present in some very special cases. Apart from performance issues, it is some sort of philosophical question, 'is it better to provide an option or have it activated for all of us'.

Gute Besserung!
Regards,
Ian

Serendipity Styx Edition and additional_plugins @ https://ophian.github.io/ @ https://github.com/ophian
mattsches
Regular
Posts: 440
Joined: Sat Nov 05, 2005 9:35 pm
Location: Wiesbaden, Germany
Contact:

Re: Smarty incompatible in S9Y 1.7?

Post by mattsches »

Just wanted to report that it's working for me on 1.7alpha (latest from git) using this code:

Code: Select all

$serendipity['smarty']->security_settings[@INCLUDE_ANY] = true;
if (version_compare($serendipity['version'], '1.7-alpha1')>=0) {
    $serendipity['smarty']->disableSecurity();
} else {
    $serendipity['smarty']->security = false;
}
$content = @$serendipity['smarty']->fetch('file:'. $tfile);//, false
$serendipity['smarty']->security_settings[@INCLUDE_ANY] = $inclusion;
I'm not sure what to make of it, but you'll certainly figure that out, right :wink:

Regards
- mattsches
Timbalu
Regular
Posts: 4598
Joined: Sun May 02, 2004 3:04 pm

Re: Smarty incompatible in S9Y 1.7?

Post by Timbalu »

Yes, before everybody gets more fuzzy about this:
  • erase the if {} else {} part.
It is not necessary! :wink:
Regards,
Ian

Serendipity Styx Edition and additional_plugins @ https://ophian.github.io/ @ https://github.com/ophian
mattsches
Regular
Posts: 440
Joined: Sat Nov 05, 2005 9:35 pm
Location: Wiesbaden, Germany
Contact:

Re: Smarty incompatible in S9Y 1.7?

Post by mattsches »

Timbalu wrote:Yes, before everybody gets more fuzzy about this:
  • erase the if {} else {} part.
It is not necessary! :wink:
But that's the point I wanted to make 8) Without the if/else, it doesn't work for me ...

Code: Select all

Fatal error: Uncaught exception 'SmartyException' with message 'directory '/var/www/vhosts/s9y.local/additional_plugins/serendipity_event_oembed/oembed.tpl' not allowed by security setting' in /var/www/vhosts/s9y.local/httpdocs/bundled-libs/Smarty/libs/sysplugins/smarty_security.php on line 375

SmartyException: directory '/var/www/vhosts/s9y.local/additional_plugins/serendipity_event_oembed/oembed.tpl' not allowed by security setting in /var/www/vhosts/s9y.local/httpdocs/bundled-libs/Smarty/libs/sysplugins/smarty_security.php on line 375
using

Code: Select all

$serendipity['smarty']->security_settings[@INCLUDE_ANY] = true;
$content = @$serendipity['smarty']->fetch('file:'. $tfile);
$serendipity['smarty']->security_settings[@INCLUDE_ANY] = $inclusion;
Maybe I got it completely wrong and you guys figured all that out before .. but I'm still confused :shock:
Timbalu
Regular
Posts: 4598
Joined: Sun May 02, 2004 3:04 pm

Re: Smarty incompatible in S9Y 1.7?

Post by Timbalu »

I need some more information
Serendipity version
Smarty version (in bundled-libs/Smarty/change_log.txt)
Did you try to clear templates_c before?
Do you get a more detailled error?

....

but wait.... Why do I see a difference in you path settings...? ;-)

/var/www/vhosts/s9y.local/additional_plugins/serendipity_event_oembed/oembed.tpl'
not allowed by security setting in
/var/www/vhosts/s9y.local/httpdocs/bundled-libs/Smarty/libs/sysplugins/smarty_security.php
on line 375

Smarty-Security does not allow path´s outside the allowed path, which is the correct behaviour. And a plugins tpl file should be called only either inside the plugins/plugin-name/ or templates/template-name/ path, in your case
/var/www/vhosts/s9y.local/httpdocs/plugins/serendipity_event_oembed/oembed.tpl'
Regards,
Ian

Serendipity Styx Edition and additional_plugins @ https://ophian.github.io/ @ https://github.com/ophian
blog.brockha.us
Regular
Posts: 695
Joined: Tue Jul 03, 2007 3:34 am
Location: Berlin, Germany
Contact:

Re: Smarty incompatible in S9Y 1.7?

Post by blog.brockha.us »

Well. Mattsches setup is not that uncommon. Even Garvin has a setup it like that. He has a different plugin path than the common.
This is (in Garvins case AFAIK) because he checked out the two repositories (s9y and add_plug) in two seperate directories of course.
- Grischa Brockhaus - http://blog.brockha.us
- Want to make me happy? http://wishes.brockha.us/
Timbalu
Regular
Posts: 4598
Joined: Sun May 02, 2004 3:04 pm

Re: Smarty incompatible in S9Y 1.7?

Post by Timbalu »

Yeah. Thats ok if you have a developers setup.
These ones should know (now) to have smarty-security disabled at all, if really going that way.
But we are coding for users to have plugins to be in /plugins, don't we? Else, we really should introduce an option for this and react on different path setting, that should not be to much work.
Regards,
Ian

Serendipity Styx Edition and additional_plugins @ https://ophian.github.io/ @ https://github.com/ophian
blog.brockha.us
Regular
Posts: 695
Joined: Tue Jul 03, 2007 3:34 am
Location: Berlin, Germany
Contact:

Re: Smarty incompatible in S9Y 1.7?

Post by blog.brockha.us »

Well, I was always forced to code plugins having this setup in mind. Garvin told me to add a configuration into the plugins I updated where developers(?) can enter the correct plugin path.

This is a discussion, Garvin should lead. I don't have this setup (I don't even know how this is done).

If this type of setup is somewhat common, I think, there should be some global s9y setting defining, where the plugin path is located instead of assuming, that it is under s9y/plugins. This path should be handed to plugins (instead of having each of them defining it again). If it is done like this, we don't have to disable security even for this type of setup. Smarty could look into this global setting, too, and add this path to the allowed tpl paths.
- Grischa Brockhaus - http://blog.brockha.us
- Want to make me happy? http://wishes.brockha.us/
Timbalu
Regular
Posts: 4598
Joined: Sun May 02, 2004 3:04 pm

Re: Smarty incompatible in S9Y 1.7?

Post by Timbalu »

Yes, thats what I meant to say! :wink:
Regards,
Ian

Serendipity Styx Edition and additional_plugins @ https://ophian.github.io/ @ https://github.com/ophian
garvinhicking
Core Developer
Posts: 30022
Joined: Tue Sep 16, 2003 9:45 pm
Location: Cologne, Germany
Contact:

Re: Smarty incompatible in S9Y 1.7?

Post by garvinhicking »

Hi!

Tough choice.

From what I read these are the options:

1. Use the proposed patch for the __set() method to be able to use the current plugin's code so that s9y disables/enables the security properly on changing the ->security_settings array. Old plugins can then use the current code, and new ones as well. Security will still work for plugins that don't disable security, and it will work for installations that rely on the 'security' option

2. Disable the s9y security alltogether so that all plugins can do whatever they like; current setups relying on that need to be told that it's no longer an option (i.e.: Remove feature)

3. Rely on the path setting for plugins like currently, but users that use symbolic link for additional plugins (like mattsches, grischa(?) and me) need to set a global configuration flag that disables security on those peoples installation at all. The disadvantage is that everyone that currently has such a setup needs to read READMEs and update their installations, while not really getting any advantage out of this, and losing security features.

The more I think about it, I think only option 1.) is the right way to go, it has the least impact, does not change things for current/old users and does not remove any functionality.

[an hour later]

This is bullshit, option 1 does not properly work; __set() does not work on setting $class->array[key] - for that to work we'd also have to implement __get() and this would then be too mch affecting performance.

I now wrote a mail to a smarty developer (hello Rodney) to see if there is any way to make PHP plugins not getting security restrictions applied, but only within a .tpl file. I hope we can work something like that out.

Regards,
Garvin
# Garvin Hicking (s9y Developer)
# Did I help you? Consider making me happy: http://wishes.garv.in/
# or use my PayPal account "paypal {at} supergarv (dot) de"
# My "other" hobby: http://flickr.garv.in/
Timbalu
Regular
Posts: 4598
Joined: Sun May 02, 2004 3:04 pm

Re: Smarty is compatible with S9Y 1.7!!!

Post by Timbalu »

Its not that much to rethink ... :wink:

The current situation:
  • 1. As known, Smarty-Security is a blowup and costs some performance
    2. As known, it is only really needed, if someone uses ftp for *.tpl files only, in /templates
    3. We have some (old) plugins with that on/off/on 5-liner, which has to be kept until there will be an official break
    4. This on/off/on 5-liner does not hurt current trunks Serendipity settings, except these 2 or three (or four) developers using a different plugin dir
    5. is there more?
What to do the (c)lean way:
  • 1. disable security for all of us, as it has no use and may open some real new features...
    2. optional have a config option for people in real need, with some notices about security
    3. Hold a property $security_setting in Serendipity_Smarty_class to avoid errors by old 5-liners
EDIT:
To keep persistence, we could implement some instructions to the upgrade procedure, so everyone has to decide using the old or the new security setting. For new installation the default is without Smarty-Security.
Regards,
Ian

Serendipity Styx Edition and additional_plugins @ https://ophian.github.io/ @ https://github.com/ophian
Post Reply