Off hand, I have no objections. Though I don't yet know exactly how you imagine it. MemberSelector.parse and MemberSelector.isIgnoredLine are public. The "@" <rule> <upperBoundType> syntax parsing is not public. Do you intend to use that in your application?
On Wed, Jan 22, 2020 at 12:45 AM Christoph Rüger <[email protected]> wrote: > Daniel, do you have any objections to refactoring the file-parsing-code in > DefaultMemberAccessPolicy to be reusable? e.g. to use it against an own > file which is structured like DefaultMemberAccessPolicy-rules? Even when > using WhitelistMemberAccessPolicy it would be great to maintain this in a > file like DefaultMemberAccessPolicy-rules > > Sure, one could write own logic like this, but why not built upon this one > if someone wants to maintain the list in a text file. > > > Am Do., 16. Jan. 2020 um 23:07 Uhr schrieb Christoph Rüger < > [email protected]>: > > > > > Am Do., 16. Jan. 2020 um 07:49 Uhr schrieb Daniel Dekany < > > [email protected]>: > > > >> 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. > >> > > > > Good idea, thanks. > > > > > >> > >> 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 > >> > > > > > > -- > > Christoph Rüger, Geschäftsführer > > Synesty <https://synesty.com/> - Anbinden und Automatisieren ohne > > Programmieren > > > > > -- > Christoph Rüger, Geschäftsführer > Synesty <https://synesty.com/> - Anbinden und Automatisieren ohne > Programmieren > > -- > 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
