Quick idea... What if you create a MemberAccessPolicy implementation that just delegates to the actual WhiltlistAccessPolicy, which is in an AtomicReference field. When something registers a new piece a whitelist, you fully recreate this embedded WhitelistAcessPolicy. I guess such even would be rare considering the whole lifecycle of the application.
On Wed, Jan 15, 2020 at 9:47 AM Christoph Rüger <[email protected]> wrote: > 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 > -- Best regards, Daniel Dekany
