Re: [FM3] Further legacy things to drop...

[Daniel Dekany]

Monday, January 16, 2017, 12:22:51 PM, Christoph Rüger wrote:

[snip]
> NodeModel.getDocumentBuilderFactory(). This is when also the
> external entities are resolved and DOM is created. So it would be
> good, if this could be customized (either by passing parameters or some other 
> mechanism).

As all the problematic methods are static, and FreeMarker doesn't even
call them anywhere, adding an overload with more parameters seems to
be the only option.

> I guess we had to do some "hacky" customizing of NodeModel in order
> to be able to use our own instance of DocumentBuilderFactory used by 
> NodeModel.

NodeModel instances don't use the DocumentBuilderFactory.

-- 
Thanks,
 Daniel Dekany

Re: [FM3] Further legacy things to drop...

[Christoph Rüger]

2017-01-16 11:58 GMT+01:00 Daniel Dekany :

> Monday, January 16, 2017, 10:05:13 AM, Christoph Rüger wrote:
>
> [snip]
> > The DocumentBuildFactory created
> > in freemarker.ext.dom.NodeModel.getDocumentBuilderFactory() is currently
> > setup to resolve external entities (which is default behavior).
>
> I believe it's pretty much necessary too. Many XML-s do need them in
> reality.
>
> > This opens
> > the door for XXE attacks (External Entity Injection): see
> > http://colesec.inventedtheinternet.com/attacking-xml-with-xml-
> external-entity-injection-xxe/
>
> That's good to keep in mind in general, but a template author isn't
> supposed to be able to provide the XML. The DOM is normally already in
> the data-model before the template processing is started. So if
> someone can only edit the template, can he still exploit this?
>
I would say no. It is not something the template author can exploit, but
more the party providing the XML.



>
> > Lot's of web-apps have this problem e.g. also PHP where this is enabled
> by
> > default too (not a php guy, but read about it).
> >
> > We basically chose to forbid ExternalEntities completely by extending
> > NodeModel and so set some properties to the DocumentBuilderFactory as
> > described here:
> > https://www.owasp.org/index.php/XML_External_Entity_(XXE)_
> Prevention_Cheat_Sheet#JAXP_DocumentBuilderFactory.2C_
> SAXParserFactory_and_DOM4J
>
> Note sure what you can do with extending NodeModel, because
> getDocumentBuilderFactory is static. As far as I see, all we (at
> FreeMarker) can do is adding a boolean disableExternalEntityResolution
> parameter to that static method and to the also static NodeModel.parse
> methods, and JavaDoc the dangers clearly there.


A parameter *disableExternalEntityResolution* would be helpful.


> I'm not aware of
> FreeMarker itself loading XML automatically (it doesn't call these
> static methods from inside), it only works with the DOM provided
> (which then already contains the external entities resolved).
>

You can pass a File to *NodeModel.parse(File f)* which is at this point not
parsed yet. The parsing happens later using the DocumentBuilderFactory
created by *NodeModel.getDocumentBuilderFactory(). *This is when also the
external entities are resolved and DOM is created. So it would be good, if
this could be customized (either by passing parameters or some other
mechanism).

I guess we had to do some "hacky" customizing of NodeModel in order to be
able to use our own instance of DocumentBuilderFactory used by NodeModel.


>
> >> >> > 2017-01-12 23:58 GMT+01:00 Daniel Dekany :
> >> >> > I have collected some further easy changes for FM3... Any comments?
> >> >> >
> >> >> > - Drop FTL classic compatible mode option (Roughly emulates FM1
> >> >> >   behavior at null-s and at some type handling issues)
> >> >> >
> >> >> > - Drop FTL non-strict syntax option (FM1 syntax - that's where you
> >> >> >   could write  instead of <#if x>).
> >> >> >
> >> >> > - Drop all the "public static void main(String[] args)" methods
> >> >> (security concern)
> >> >> >
> >> >> > - Drop freemarker.log. That's a simple log adapter facility from
> the
> >> >> >   ancient times of Java, kind of like commons-logging or slf4j. I
> >> >> >   would instead introduce slf4j-api as a required dependency.
> >> >> >
> >> >> > - Drop legacy XML wrapper (freemarker.ext.xml, not to be confused
> with
> >> >> >   freemarker.ext.dom)
> >> >> >
> >> >> > - Drop ant task (freemarker.ext.ant)
> >> >> >
> >> >> > --
> >> >> > Thanks,
> >> >> >  Daniel Dekany
> >> >> >
> >> >> >
> >> >> >
> >> >> >
> >> >>
> >> >> --
> >> >> Thanks,
> >> >>  Daniel Dekany
> >> >>
> >> >>
> >> >
> >> >
> >> > --
> >> > Christoph Rüger, Geschäftsführer
> >> > Synesty  - Automatisierung, Schnittstellen,
> >> Datenfeeds
> >> > Tel.: +49 3641/559649
> >> >
> >> > Xing: https://www.xing.com/profile/Christoph_Rueger2
> >> > LinkedIn: http://www.linkedin.com/pub/christoph-rueger/a/685/198
> >> >
> >>
> >> --
> >> Thanks,
> >>  Daniel Dekany
> >>
> >>
> >
> >
> > --
> > Christoph Rüger, Geschäftsführer
> > Synesty  - Automatisierung, Schnittstellen,
> Datenfeeds
> > Tel.: +49 3641/559649 <+49%203641%20559649>
> >
> > Xing: https://www.xing.com/profile/Christoph_Rueger2
> > LinkedIn: http://www.linkedin.com/pub/christoph-rueger/a/685/198
> >
>
> --
> Thanks,
>  Daniel Dekany
>
>


-- 
Christoph Rüger, Geschäftsführer
Synesty  - Automatisierung, Schnittstellen, Datenfeeds
Tel.: +49 3641/559649

Xing: https://www.xing.com/profile/Christoph_Rueger2
LinkedIn: http://www.linkedin.com/pub/christoph-rueger/a/685/198

-- 
Synesty GmbH
Moritz-von-Rohr-Str. 1a
07745 Jena
Tel.: +49 3641 559649
Fax.: +49 3641 5596499
Internet: http://synesty.com

Geschäftsführer: Christoph Rüger
Unternehmenssitz: Jena
Handelsregister B beim Amtsgericht: Jena
Handelsregister-Nummer: HRB 508766
Ust-IdNr.: 

Re: [FM3] Further legacy things to drop...

[Daniel Dekany]

Friday, January 13, 2017, 1:07:36 PM, Christoph Rüger wrote:

> 2017-01-13 1:17 GMT+01:00 Daniel Dekany :
>
>> Friday, January 13, 2017, 12:08:12 AM, Christoph Rüger wrote:
>>
>> > +1 for everything.
>> >
>> > additional security topics:
>> > use TemplateClassResolver.ALLOWS_NOTHING_RESOLVER by default to
>> > avoid template injection attacks.
>>
>> At least in FM2 you pull in your TemplateDirectiveModel-s and
>> TemplateMethodModel-s into #import/#include-able FTL-s with `?new`. I
>> can imagine much better mechanisms for that use-case though... But for
>> now, the point is that we can't just default to
>> ALLOWS_NOTHING_RESOLVER without giving an alternative first. But, now
>> that you say, I will delete those legacy "utility" TemplateModel-s
>> which make `?new` rather dangerous.
>>
>
> Yes, that's what I meant. e.g. "Execute
> "
> where can run code on the server:
>
> <#assign ex="freemarker.template.utility.Execute"?new()> ${ ex("id")}

Yeah, that's my favorite... Obviously, the mind set back then was that
templates are just part of the source code like java files are. You
can do whatever you want in both.

> Source
> 
>
> I have another thing regarding XXE-Attacks in FM-XML-processing
> (regarding DocumentBuilderFactory
> in freemarker.ext.dom.NodeModel) where a different default behavior would
> be good IMO.
> I can give more details in a separate email if you want.

Please do! That affects FM2 too.

>> > 2017-01-12 23:58 GMT+01:00 Daniel Dekany :
>> > I have collected some further easy changes for FM3... Any comments?
>> >
>> > - Drop FTL classic compatible mode option (Roughly emulates FM1
>> >   behavior at null-s and at some type handling issues)
>> >
>> > - Drop FTL non-strict syntax option (FM1 syntax - that's where you
>> >   could write  instead of <#if x>).
>> >
>> > - Drop all the "public static void main(String[] args)" methods
>> (security concern)
>> >
>> > - Drop freemarker.log. That's a simple log adapter facility from the
>> >   ancient times of Java, kind of like commons-logging or slf4j. I
>> >   would instead introduce slf4j-api as a required dependency.
>> >
>> > - Drop legacy XML wrapper (freemarker.ext.xml, not to be confused with
>> >   freemarker.ext.dom)
>> >
>> > - Drop ant task (freemarker.ext.ant)
>> >
>> > --
>> > Thanks,
>> >  Daniel Dekany
>> >
>> >
>> >
>> >
>>
>> --
>> Thanks,
>>  Daniel Dekany
>>
>>
>
>
> -- 
> Christoph Rüger, Geschäftsführer
> Synesty  - Automatisierung, Schnittstellen, Datenfeeds
> Tel.: +49 3641/559649
>
> Xing: https://www.xing.com/profile/Christoph_Rueger2
> LinkedIn: http://www.linkedin.com/pub/christoph-rueger/a/685/198
>

-- 
Thanks,
 Daniel Dekany

Re: [FM3] Further legacy things to drop...

[Christoph Rüger]

2017-01-13 1:17 GMT+01:00 Daniel Dekany :

> Friday, January 13, 2017, 12:08:12 AM, Christoph Rüger wrote:
>
> > +1 for everything.
> >
> > additional security topics:
> > use TemplateClassResolver.ALLOWS_NOTHING_RESOLVER by default to
> > avoid template injection attacks.
>
> At least in FM2 you pull in your TemplateDirectiveModel-s and
> TemplateMethodModel-s into #import/#include-able FTL-s with `?new`. I
> can imagine much better mechanisms for that use-case though... But for
> now, the point is that we can't just default to
> ALLOWS_NOTHING_RESOLVER without giving an alternative first. But, now
> that you say, I will delete those legacy "utility" TemplateModel-s
> which make `?new` rather dangerous.
>

Yes, that's what I meant. e.g. "Execute
"
where can run code on the server:

<#assign ex="freemarker.template.utility.Execute"?new()> ${ ex("id")}
Source


I have another thing regarding XXE-Attacks in FM-XML-processing
(regarding DocumentBuilderFactory
in freemarker.ext.dom.NodeModel) where a different default behavior would
be good IMO.
I can give more details in a separate email if you want.



> > 2017-01-12 23:58 GMT+01:00 Daniel Dekany :
> > I have collected some further easy changes for FM3... Any comments?
> >
> > - Drop FTL classic compatible mode option (Roughly emulates FM1
> >   behavior at null-s and at some type handling issues)
> >
> > - Drop FTL non-strict syntax option (FM1 syntax - that's where you
> >   could write  instead of <#if x>).
> >
> > - Drop all the "public static void main(String[] args)" methods
> (security concern)
> >
> > - Drop freemarker.log. That's a simple log adapter facility from the
> >   ancient times of Java, kind of like commons-logging or slf4j. I
> >   would instead introduce slf4j-api as a required dependency.
> >
> > - Drop legacy XML wrapper (freemarker.ext.xml, not to be confused with
> >   freemarker.ext.dom)
> >
> > - Drop ant task (freemarker.ext.ant)
> >
> > --
> > Thanks,
> >  Daniel Dekany
> >
> >
> >
> >
>
> --
> Thanks,
>  Daniel Dekany
>
>


-- 
Christoph Rüger, Geschäftsführer
Synesty  - Automatisierung, Schnittstellen, Datenfeeds
Tel.: +49 3641/559649

Xing: https://www.xing.com/profile/Christoph_Rueger2
LinkedIn: http://www.linkedin.com/pub/christoph-rueger/a/685/198

-- 
Synesty GmbH
Moritz-von-Rohr-Str. 1a
07745 Jena
Tel.: +49 3641 559649
Fax.: +49 3641 5596499
Internet: http://synesty.com

Geschäftsführer: Christoph Rüger
Unternehmenssitz: Jena
Handelsregister B beim Amtsgericht: Jena
Handelsregister-Nummer: HRB 508766
Ust-IdNr.: DE287564982

Re: [FM3] Further legacy things to drop...

[Daniel Dekany]

Yeah, that's one of the parser-related wishes I saw for a few time. I
heads it's also possible with JavaCC. This would be also very useful
for IDE plugins (the current Eclipse plugin only marks the first
error). Anyway, when we get to the template language issues, we shall
see what direction that will take (like if the parser will be quite
different anyway).


Friday, January 13, 2017, 12:37:55 AM, Jaime Garza wrote:

> It would be awesome if the parser was able to recover on error and
> return multiple errors, instead of the first one. Perhaps you may
> need to use a bottoms-up parser instead of javacc. My customers
> complain that they need to try the “run” (I use a preview mode
> actually) multiple times if they have multiple errors.
>
> Jaime
>
>> On Jan 12, 2017, at 3:10 PM, David E Jones  wrote:
>> 
>> 
>> These look good to me. Always feels good to clean out old and unneeded code. 
>> :)
>> 
>> -David
>> 
>> 
>> On Thu, 2017-01-12 at 23:58 +0100, Daniel Dekany wrote:
>>> I have collected some further easy changes for FM3... Any comments?
>>> 
>>> - Drop FTL classic compatible mode option (Roughly emulates FM1
>>>   behavior at null-s and at some type handling issues)
>>> 
>>> - Drop FTL non-strict syntax option (FM1 syntax - that's where you
>>>   could write  instead of <#if x>).
>>> 
>>> - Drop all the "public static void main(String[] args)" methods (security 
>>> concern)
>>> 
>>> - Drop freemarker.log. That's a simple log adapter facility from the
>>>   ancient times of Java, kind of like commons-logging or slf4j. I
>>>   would instead introduce slf4j-api as a required dependency.
>>> 
>>> - Drop legacy XML wrapper (freemarker.ext.xml, not to be confused with
>>>   freemarker.ext.dom)
>>> 
>>> - Drop ant task (freemarker.ext.ant)
>>> 
>
>

-- 
Thanks,
 Daniel Dekany

Re: [FM3] Further legacy things to drop...

[Daniel Dekany]

Friday, January 13, 2017, 12:08:12 AM, Christoph Rüger wrote:

> +1 for everything. 
>
> additional security topics:
> use TemplateClassResolver.ALLOWS_NOTHING_RESOLVER by default to
> avoid template injection attacks. 

At least in FM2 you pull in your TemplateDirectiveModel-s and
TemplateMethodModel-s into #import/#include-able FTL-s with `?new`. I
can imagine much better mechanisms for that use-case though... But for
now, the point is that we can't just default to
ALLOWS_NOTHING_RESOLVER without giving an alternative first. But, now
that you say, I will delete those legacy "utility" TemplateModel-s
which make `?new` rather dangerous.

> 2017-01-12 23:58 GMT+01:00 Daniel Dekany :
> I have collected some further easy changes for FM3... Any comments?
>
> - Drop FTL classic compatible mode option (Roughly emulates FM1
>   behavior at null-s and at some type handling issues)
>
> - Drop FTL non-strict syntax option (FM1 syntax - that's where you
>   could write  instead of <#if x>).
>
> - Drop all the "public static void main(String[] args)" methods (security 
> concern)
>
> - Drop freemarker.log. That's a simple log adapter facility from the
>   ancient times of Java, kind of like commons-logging or slf4j. I
>   would instead introduce slf4j-api as a required dependency.
>
> - Drop legacy XML wrapper (freemarker.ext.xml, not to be confused with
>   freemarker.ext.dom)
>
> - Drop ant task (freemarker.ext.ant)
>
> --
> Thanks,
>  Daniel Dekany
>
>
>
>

-- 
Thanks,
 Daniel Dekany

Re: [FM3] Further legacy things to drop...

[Jaime Garza]

It would be awesome if the parser was able to recover on error and return 
multiple errors, instead of the first one. Perhaps you may need to use a 
bottoms-up parser instead of javacc. My customers complain that they need to 
try the “run” (I use a preview mode actually) multiple times if they have 
multiple errors.

Jaime

> On Jan 12, 2017, at 3:10 PM, David E Jones  wrote:
> 
> 
> These look good to me. Always feels good to clean out old and unneeded code. 
> :)
> 
> -David
> 
> 
> On Thu, 2017-01-12 at 23:58 +0100, Daniel Dekany wrote:
>> I have collected some further easy changes for FM3... Any comments?
>> 
>> - Drop FTL classic compatible mode option (Roughly emulates FM1
>>   behavior at null-s and at some type handling issues)
>> 
>> - Drop FTL non-strict syntax option (FM1 syntax - that's where you
>>   could write  instead of <#if x>).
>> 
>> - Drop all the "public static void main(String[] args)" methods (security 
>> concern)
>> 
>> - Drop freemarker.log. That's a simple log adapter facility from the
>>   ancient times of Java, kind of like commons-logging or slf4j. I
>>   would instead introduce slf4j-api as a required dependency.
>> 
>> - Drop legacy XML wrapper (freemarker.ext.xml, not to be confused with
>>   freemarker.ext.dom)
>> 
>> - Drop ant task (freemarker.ext.ant)
>> 

Re: [FM3] Further legacy things to drop...

[David E Jones]

These look good to me. Always feels good to clean out old and unneeded code. :)

-David


On Thu, 2017-01-12 at 23:58 +0100, Daniel Dekany wrote:
> I have collected some further easy changes for FM3... Any comments?
> 
> - Drop FTL classic compatible mode option (Roughly emulates FM1
>   behavior at null-s and at some type handling issues)
> 
> - Drop FTL non-strict syntax option (FM1 syntax - that's where you
>   could write  instead of <#if x>).
> 
> - Drop all the "public static void main(String[] args)" methods (security 
> concern)
> 
> - Drop freemarker.log. That's a simple log adapter facility from the
>   ancient times of Java, kind of like commons-logging or slf4j. I
>   would instead introduce slf4j-api as a required dependency.
> 
> - Drop legacy XML wrapper (freemarker.ext.xml, not to be confused with
>   freemarker.ext.dom)
> 
> - Drop ant task (freemarker.ext.ant)
> 

Re: [FM3] Further legacy things to drop...

[Christoph Rüger]

+1 for everything.

additional security topics:
use TemplateClassResolver.ALLOWS_NOTHING_RESOLVER by default to avoid
template injection attacks.



2017-01-12 23:58 GMT+01:00 Daniel Dekany :

> I have collected some further easy changes for FM3... Any comments?
>
> - Drop FTL classic compatible mode option (Roughly emulates FM1
>   behavior at null-s and at some type handling issues)
>
> - Drop FTL non-strict syntax option (FM1 syntax - that's where you
>   could write  instead of <#if x>).
>
> - Drop all the "public static void main(String[] args)" methods (security
> concern)
>
> - Drop freemarker.log. That's a simple log adapter facility from the
>   ancient times of Java, kind of like commons-logging or slf4j. I
>   would instead introduce slf4j-api as a required dependency.
>
> - Drop legacy XML wrapper (freemarker.ext.xml, not to be confused with
>   freemarker.ext.dom)
>
> - Drop ant task (freemarker.ext.ant)
>
> --
> Thanks,
>  Daniel Dekany
>
>


-- 
Christoph Rüger, Geschäftsführer
Synesty  - Automatisierung, Schnittstellen, Datenfeeds
Tel.: +49 3641/559649

Xing: https://www.xing.com/profile/Christoph_Rueger2
LinkedIn: http://www.linkedin.com/pub/christoph-rueger/a/685/198

-- 
Synesty GmbH
Moritz-von-Rohr-Str. 1a
07745 Jena
Tel.: +49 3641 559649
Fax.: +49 3641 5596499
Internet: http://synesty.com

Geschäftsführer: Christoph Rüger
Unternehmenssitz: Jena
Handelsregister B beim Amtsgericht: Jena
Handelsregister-Nummer: HRB 508766
Ust-IdNr.: DE287564982