First of all, great stuff. Also thanks for making BeansWrapper.invokeMethod(Object, Method, Object[]) protected, as this helps us to monitor method invocations. As you write in a comment it will be "significant work to put together" a whitelist, but this will help to do so. Do you think it makes sense to provide a helper method e.g. public String MemberSelector.toSelectorRulesString() which outputs a String which is understood by MemberSelector.parse(String)? Could be helpful for monitoring in that context to make sure you create such rules (strings) and always get the syntax right.
Am Di., 14. Jan. 2020 um 23:40 Uhr schrieb Daniel Dekany < [email protected]>: > And updated it again... I hope I won't find any more things I missed to > address. > > Anyway, I think we should start going for a release (in a month or > something), so, Christoph, any idea when can you say something about the > OSGi issues? I don't want to release something where that can't be solved. > I had a first look at it and try to wrap my head around it. Regarding OSGI: I noticed that a Classloader can be passed to e.g. MemberSelector.parse(Collection<String>, boolean, ClassLoader) which is always a good thing for OSGI. The key thing in OSG is that new Classes (provided by bundles) can appear dynamically at runtime at any point in time. So I think we would need to add rules to MemberAccessPolicy dynamically. Since MemberSelectorListMemberAccessPolicy.forClass(Class<?>) is made final I assume we need to write our own MemberAccessPolicy from scratch (or duplicate code from MemberSelectorListMemberAccessPolicy) in order to add MemberSelectors dynamically. Right? Or would it be possible to somehow extend MemberSelectorListMemberAccessPolicy / WhitelistMemberAccessPolicy and add MemberSelectors to the internal matchers (e.g. MethodMatcher etc.) from a subclass? I guess we would like to subclass WhitelistMemberAccessPolicy to handle dynamic registration of our OSGI stuff (means adding MemberSelectors dynamically). It might be too early as I have not fully understood everything, but maybe you can provide first thoughts. > On Sat, Jan 11, 2020 at 8:25 PM Daniel Dekany <[email protected]> > wrote: > > > I have also updated the default member access policy, so the tricks you > > tried back then shouldn't work anymore, even if you don't use your own > > member access policy. But, you still definitely should use your own > policy, > > if users aren't trusted. > > > > The other API-s and Javadocs were evolved too a bit since then; I have > > deployed it to the maven repo and updated > > https://freemarker.apache.org/builds/fm2 > > < > https://freemarker.apache.org/builds/fm2/api/freemarker/ext/beans/MemberAccessPolicy.html > > > > accordingly, > > > > > > On Thu, Jan 2, 2020 at 10:55 PM Christoph Rüger <[email protected]> > > wrote: > > > >> Am Mi., 1. Jan. 2020 um 22:12 Uhr schrieb Daniel Dekany < > >> [email protected]>: > >> > >> > Guys, > >> > > >> > I have add MemberAccessPolicy to the API, which you can plug into a > >> > DefaultObjectWrapper (or to any BeansWrapper). I have also added > >> > WhitelistMemberAccessPolicy, to ease adding a restrictive policy. > Please > >> > take a look. 2.3.30-SNAPSHOT is in the Apache snapshot repo, as usual. > >> You > >> > can start out from here in API documentation: > >> > > >> > > >> > https://freemarker.apache.org/builds/fm2/api/freemarker/ext/beans/MemberAccessPolicy.html > >> > >> > >> Thanks Daniel and happy new year :) > >> We will try to test this. Cannot promise how soon we get to it, but I > will > >> try my best. > >> We will also check how this behaves in our OSGI world. > >> > >> > >> > > >> > > >> > So please review these, tell if you have any recommendations, or you > >> see a > >> > way to circumvent this. (One risky thing I see is that we have a long > >> > deprecated default static instance of DefaultObjectWrapper, which if > >> course > >> > doesn't use any custom MemberAccessPolicy. We use that static instance > >> > internally in FM2 on a lot of places. I will have to review all such > >> cases, > >> > and also make it less probable that they can become exploitable > later.) > >> > > >> > I will also create a new implementation for DefaultMemberAccessPolicy > >> > later. The current one does exactly what the old one did. The only > real > >> > solution will be still WhitelistMemberAccessPolicy, if someone indeed > >> > doesn't trust the template authors. > >> > > >> > On Tue, Dec 24, 2019 at 6:31 PM Siegfried Goeschl < > >> > [email protected]> wrote: > >> > > >> > > HI Daniel, > >> > > > >> > > Would it make sense to come up with a separate chapter for the > >> existing > >> > > FreeMarker documentation to explain the things in detail? > >> > > > >> > > Thanks in advance, > >> > > > >> > > Siegfried Goeschl > >> > > > >> > > PS: Last email for today - preparing Christmas dinner :-) > >> > > > >> > > > On 24.12.2019, at 18:23, Daniel Dekany <[email protected]> > >> > wrote: > >> > > > > >> > > > I responded to your mail that says "The problem described in the > >> > article > >> > > > was less about arbitrary people but someone who hacked through > other > >> > > > security issues to become administrator with temple editing > rights". > >> > So I > >> > > > thought that the premise there was that people who shouldn't be > >> able to > >> > > > edit templates become able to do so. But it doesn't mater how it > >> was in > >> > > > that case. Because as I said in the linked bug report, and as the > >> FAQ > >> > > says, > >> > > > if you allow someone to edit templates with the default FreeMarker > >> > > > configuration, that's almost like if you allow them to edit Java > >> files. > >> > > So > >> > > > whatever your application has right to do (like read the password > >> file, > >> > > > launch missiles, etc.), the templates probably can do as well. The > >> > point > >> > > of > >> > > > discouraging complex/technical logic in templates (not just in > >> > > FreeMarker) > >> > > > was the MVC principle, where you should only put presentation > logic > >> > into > >> > > > the templates. > >> > > > > >> > > > We can't provide a practically useful default configuration that's > >> > secure > >> > > > if you can't trust people that can edit templates, because the > >> > whitelist > >> > > > content is specific to the concrete application. By default ?new > is > >> not > >> > > > restricted (well, it can instantiate TemplatModel-s only, but that > >> > hardly > >> > > > saves anyone security wise). The reason ?api is still disabled by > >> > default > >> > > > is that if someone went through the pain of setting up FreeMarker > >> to be > >> > > > safe(r) (which implies that you do not use the default > >> ObjectWrapper, > >> > nor > >> > > > the default settings for ?new, and you are thoughtful with your > >> > > > TemplateLoader, as the FAQ says), then the new FreeMarker version > >> where > >> > > > ?api was introduced should not open a new hole on your system. For > >> > almost > >> > > > all users though, ?api enabled by default would be better (it's > >> mostly > >> > to > >> > > > allow users to work around TemplateHashModel limitations when > >> dealing > >> > > with > >> > > > java.util.Map-s), but I have chosen the safer approach when I > added > >> it. > >> > > > > >> > > > The unsafeMethods mechanism will be updated, as things stand, > >> despite > >> > > that > >> > > > it's not strictly backward compatible. It will be still a quite > >> > pointless > >> > > > mechanism. I don't know why was it added by the author (some 10-15 > >> > years > >> > > > ago, I think). > >> > > > > >> > > > On Tue, Dec 24, 2019 at 4:14 PM Siegfried Goeschl < > >> > > > [email protected]> wrote: > >> > > > > >> > > >> Hi Daniel, > >> > > >> > >> > > >> Not sure about your line of thoughts :-) > >> > > >> > >> > > >> My understanding > >> > > >> > >> > > >> * There is a recipe out there how someone can access the file > >> system > >> > and > >> > > >> the setup was not bad security-wise - only "?api" built-in was > >> enabled > >> > > >> * I think the "?api.class.getResource" and > >> > > >> "?api.class.getResourceAsStream" can be marked as unsafe method? > >> > > >> * I also think that ALLOWS_NOTHING_RESOLVER is not the default > >> > > >> configuration? > >> > > >> * I actually tried the published code and it reads my > >> "/etc/passwd" :( > >> > > >> > >> > > >> If the assumptions above are correct - can this particular attack > >> be > >> > > >> avoided? If so we should react and improve the configuration ... > >> > > >> > >> > > >> Thanks in advance, > >> > > >> > >> > > >> Siegfried Goeschl > >> > > >> > >> > > >> > >> > > >>> On 24.12.2019, at 11:50, Daniel Dekany <[email protected] > > > >> > > wrote: > >> > > >>> > >> > > >>> The blog entry might starts its case with a privilege escalation > >> > > >>> independent of FreeMarker, but the question you got during your > >> > > >>> presentation wasn't about that, I think. But more importantly, > >> some > >> > > real > >> > > >>> world applications do allow editing templates for users who > aren't > >> > > >>> necessarily some kind of superusers. Right now, after they > >> realized > >> > > that > >> > > >>> the problem exists at all, they will have to figure out a > solution > >> > > >>> themselves. We are in a much better position to do the same. > >> > > >>> > >> > > >>> DOS-ing is certainly less of a concern in general, though > >> > unintentional > >> > > >>> DOS-ing (or I guess unintentional) was a problem for > >> > > >>> try.freemarker.apache.org in the past. My point there is just > >> that > >> > if > >> > > >>> really everyone from the Internet can edit templates, then it > will > >> > be a > >> > > >>> problem, I guess for any practical template language. > >> > > >>> > >> > > >>> > >> > > >>> On Mon, Dec 23, 2019 at 11:55 PM Siegfried Goeschl < > >> > > >>> [email protected]> wrote: > >> > > >>> > >> > > >>>> Hi Daniel, > >> > > >>>> > >> > > >>>> I guess I need to re-read the FreeMarker documentation and > >> ticket - > >> > > >> having > >> > > >>>> said that > >> > > >>>> > >> > > >>>> * The problem described in the article was less about arbitrary > >> > people > >> > > >> but > >> > > >>>> someone who hacked through other security issues to become > >> > > administrator > >> > > >>>> with temple editing rights > >> > > >>>> * The people having that skills usually don't have any interest > >> in > >> > > >>>> starting a DOS attack by messing up templates - there are more > >> > > valuable > >> > > >>>> things out there ... > >> > > >>>> * I think it is pretty much impossible to make FreeMarker 100% > >> > bullet > >> > > >>>> proof (tons of features, a lot of code, arbitrary libraries > >> coming > >> > > from > >> > > >> the > >> > > >>>> application) but at least we can check that this attack does > not > >> > work > >> > > >> any > >> > > >>>> longer > >> > > >>>> * From my understanding - usually there a couple of security > >> > > >>>> vulnerabilites leading to complete data breach :-) > >> > > >>>> > >> > > >>>> Thanks in advance, > >> > > >>>> > >> > > >>>> Siegfried Goeschl > >> > > >>>> > >> > > >>>> > >> > > >>>>> On 23.12.2019, at 22:30, Daniel Dekany < > [email protected] > >> > > >> > > >> wrote: > >> > > >>>>> > >> > > >>>>> Hi, > >> > > >>>>> > >> > > >>>>> In short, allowing untrusted users to edit templates is not > >> > > supported. > >> > > >>>> But, > >> > > >>>>> since people do it anyway, 2.3.30 will make an effort to allow > >> > doing > >> > > >> that > >> > > >>>>> with taking far less risk than what people take now. The > >> > > >>>> MemberAccessPolicy > >> > > >>>>> feature committed in recent days is the start of that. > Actually, > >> > you > >> > > >>>> could > >> > > >>>>> always just use SimpleObjectWrapper (as the FAQ states), but > >> > clearly > >> > > >>>> that's > >> > > >>>>> too limiting for what many (most?) people use FreeMarker for. > >> But > >> > > >>>> anyway, I > >> > > >>>>> don't believe that a template engine with the complexity of > >> > > FreeMarker > >> > > >>>> will > >> > > >>>>> be ever a good fit for applications where random people can > edit > >> > > >>>> templates. > >> > > >>>>> If users are accountable in real life for what they did, like > >> they > >> > > are > >> > > >>>>> employees at the client, then probably it will be good enough, > >> but > >> > > not > >> > > >>>> for > >> > > >>>>> use cases where anyone can edit templates. If nothing else, > you > >> > will > >> > > be > >> > > >>>> too > >> > > >>>>> easily DOS-able then. > >> > > >>>>> > >> > > >>>>> As of the blog entry, see this: > >> > > >>>>> https://issues.apache.org/jira/browse/FREEMARKER-124 > >> > > >>>>> Here I would add that it's likely that the calls used in the > >> blog > >> > > entry > >> > > >>>>> won't work anymore in 2.3.30. I'm a bit uneasy about that, as > >> it's > >> > a > >> > > >>>>> backward compatibility risk (it won't be just blocking that > >> single > >> > > >>>> method), > >> > > >>>>> while it doesn't provide real security. You need a whitelist > of > >> > > what's > >> > > >>>>> allowed for that (as opposed to a blacklist), and that's > >> possible > >> > to > >> > > do > >> > > >>>>> with MemberAccessPolicy, but I will also provide an > >> implementation > >> > to > >> > > >>>> help > >> > > >>>>> doing that. > >> > > >>>>> > >> > > >>>>> Also, in the FAQ: > >> > > >>>>> > >> > > >>>> > >> > > >> > >> > > > >> > > >> > https://freemarker.apache.org/docs/app_faq.html#faq_template_uploading_security > >> > > >>>>> > >> > > >>>>> > >> > > >>>>> On Mon, Dec 23, 2019 at 7:25 PM Siegfried Goeschl < > >> > > >>>>> [email protected]> wrote: > >> > > >>>>> > >> > > >>>>>> Hi folks, > >> > > >>>>>> > >> > > >>>>>> During my last presentation I was asked about how secure > Apache > >> > > >>>> Freemarker > >> > > >>>>>> is in the context of user editing their templates - well, > hard > >> to > >> > > say > >> > > >>>>>> without knowing the application. > >> > > >>>>>> > >> > > >>>>>> But I came across an interesting article (see > >> > > >>>>>> > >> https://ackcent.com/blog/in-depth-freemarker-template-injection/) > >> > > >> where > >> > > >>>>>> the authors successfully hacked a CMS based on Apache > >> FreeMarker > >> > > >>>>>> > >> > > >>>>>> * As far as I know the UNRESTRICTED_RESOLVER is the default? > >> Maybe > >> > > >>>>>> ALLOWS_NOTHING_RESOLVER would be a better default? > >> > > >>>>>> * Enabling "?api" needs to be enabled by developers which is > >> fine > >> > > >>>>>> * Update the "unsafeMethods.properties" according to the > >> article? > >> > > For > >> > > >>>> the > >> > > >>>>>> records "java.lang.Thread.suspend()" is duplicated anyway > >> > > >>>>>> > >> > > >>>>>> Thanks in advance, > >> > > >>>>>> > >> > > >>>>>> Siegfried Goeschl > >> > > >>>>>> > >> > > >>>>>> > >> > > >>>>> > >> > > >>>>> -- > >> > > >>>>> Best regards, > >> > > >>>>> Daniel Dekany > >> > > >>>> > >> > > >>>> > >> > > >>> > >> > > >>> -- > >> > > >>> Best regards, > >> > > >>> Daniel Dekany > >> > > >> > >> > > >> > >> > > > > >> > > > -- > >> > > > Best regards, > >> > > > Daniel Dekany > >> > > > >> > > > >> > > >> > -- > >> > Best regards, > >> > Daniel Dekany > >> > > >> > >> -- > >> Synesty GmbH > >> Moritz-von-Rohr-Str. 1a > >> 07745 Jena > >> Tel.: +49 3641 > >> 5596493Internet: https://synesty.com <https://synesty.com> > >> Informationen > >> zum Datenschutz: https://synesty.com/datenschutz > >> <https://synesty.com/datenschutz> > >> > >> Geschäftsführer: Christoph Rüger > >> > >> Unternehmenssitz: Jena > >> Handelsregister B beim Amtsgericht: Jena > >> > >> Handelsregister-Nummer: HRB 508766 > >> Ust-IdNr.: DE287564982 > >> > > > > > > -- > > Best regards, > > Daniel Dekany > > > > > -- > Best regards, > Daniel Dekany > -- Synesty GmbH Moritz-von-Rohr-Str. 1a 07745 Jena Tel.: +49 3641 5596493Internet: https://synesty.com <https://synesty.com> Informationen zum Datenschutz: https://synesty.com/datenschutz <https://synesty.com/datenschutz> Geschäftsführer: Christoph Rüger Unternehmenssitz: Jena Handelsregister B beim Amtsgericht: Jena Handelsregister-Nummer: HRB 508766 Ust-IdNr.: DE287564982
