You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@freemarker.apache.org by dd...@apache.org on 2020/10/23 22:02:36 UTC

[freemarker] 01/02: Added DOMNodeSupport and JythonSupport boolean properties to DefaultObjectWrapper. This allows disabling the special wrapping of DOM nodes and Jython classes. This might be desirable for security reasons.

This is an automated email from the ASF dual-hosted git repository.

ddekany pushed a commit to branch 2.3-gae
in repository https://gitbox.apache.org/repos/asf/freemarker.git

commit 133cc44da897bde4375dc0b6b2a2844ef3400fd8
Author: ddekany <dd...@apache.org>
AuthorDate: Fri Oct 23 23:53:13 2020 +0200

    Added DOMNodeSupport and JythonSupport boolean properties to DefaultObjectWrapper. This allows disabling the special wrapping of DOM nodes and Jython classes. This might be desirable for security reasons.
---
 .../freemarker/template/DefaultObjectWrapper.java  | 82 ++++++++++++++++++----
 .../DefaultObjectWrapperConfiguration.java         | 30 +++++++-
 src/manual/en_US/book.xml                          | 34 +++++----
 .../template/DefaultObjectWrapperTest.java         | 45 +++++++++++-
 4 files changed, 164 insertions(+), 27 deletions(-)

diff --git a/src/main/java/freemarker/template/DefaultObjectWrapper.java b/src/main/java/freemarker/template/DefaultObjectWrapper.java
index c0337db..eb44ce6 100644
--- a/src/main/java/freemarker/template/DefaultObjectWrapper.java
+++ b/src/main/java/freemarker/template/DefaultObjectWrapper.java
@@ -73,8 +73,10 @@ public class DefaultObjectWrapper extends freemarker.ext.beans.BeansWrapper {
     private boolean useAdaptersForContainers;
     private boolean forceLegacyNonListCollections;
     private boolean iterableSupport;
+    private boolean domNodeSupport;
+    private boolean jythonSupport;
     private final boolean useAdapterForEnumerations;
-    
+
     /**
      * Creates a new instance with the incompatible-improvements-version specified in
      * {@link Configuration#DEFAULT_INCOMPATIBLE_IMPROVEMENTS}.
@@ -134,6 +136,8 @@ public class DefaultObjectWrapper extends freemarker.ext.beans.BeansWrapper {
                 && getIncompatibleImprovements().intValue() >= _TemplateAPI.VERSION_INT_2_3_26;
         forceLegacyNonListCollections = dowDowCfg.getForceLegacyNonListCollections();
         iterableSupport = dowDowCfg.getIterableSupport();
+        domNodeSupport = dowDowCfg.getDOMNodeSupport();
+        jythonSupport = dowDowCfg.getJythonSupport();
         finalizeConstruction(writeProtected);
     }
 
@@ -255,9 +259,10 @@ public class DefaultObjectWrapper extends freemarker.ext.beans.BeansWrapper {
      * Called for an object that isn't considered to be of a "basic" Java type, like for an application specific type,
      * or for a W3C DOM node. In its default implementation, W3C {@link Node}-s will be wrapped as {@link NodeModel}-s
      * (allows DOM tree traversal), Jython objects will be delegated to the {@code JythonWrapper}, others will be
-     * wrapped using {@link BeansWrapper#wrap(Object)}. Note that if {@link #getMemberAccessPolicy()} doesn't return
-     * a {@link DefaultMemberAccessPolicy} or {@link LegacyDefaultMemberAccessPolicy}, then Jython wrapper will be
-     * skipped for security reasons.
+     * wrapped using {@link BeansWrapper#wrap(Object)}. However, these can be turned off with the
+     * {@link #setDOMNodeSupport(boolean)} and {@link #setJythonSupport(boolean)}. Note that if
+     * {@link #getMemberAccessPolicy()} doesn't return a {@link DefaultMemberAccessPolicy} or
+     * {@link LegacyDefaultMemberAccessPolicy}, then Jython wrapper will be skipped for security reasons.
      * 
      * <p>
      * When you override this method, you should first decide if you want to wrap the object in a custom way (and if so
@@ -265,16 +270,20 @@ public class DefaultObjectWrapper extends freemarker.ext.beans.BeansWrapper {
      * behavior is fine with you).
      */
     protected TemplateModel handleUnknownType(Object obj) throws TemplateModelException {
-        if (obj instanceof Node) {
+        if (domNodeSupport && obj instanceof Node) {
             return wrapDomNode(obj);
         }
-        MemberAccessPolicy memberAccessPolicy = getMemberAccessPolicy();
-        if (memberAccessPolicy instanceof DefaultMemberAccessPolicy
-                || memberAccessPolicy instanceof LegacyDefaultMemberAccessPolicy) {
-            if (JYTHON_WRAPPER != null && JYTHON_OBJ_CLASS.isInstance(obj)) {
-                return JYTHON_WRAPPER.wrap(obj);
+
+        if (jythonSupport) {
+            MemberAccessPolicy memberAccessPolicy = getMemberAccessPolicy();
+            if (memberAccessPolicy instanceof DefaultMemberAccessPolicy
+                    || memberAccessPolicy instanceof LegacyDefaultMemberAccessPolicy) {
+                if (JYTHON_WRAPPER != null && JYTHON_OBJ_CLASS.isInstance(obj)) {
+                    return JYTHON_WRAPPER.wrap(obj);
+                }
             }
         }
+
         return super.wrap(obj); 
     }
     
@@ -389,6 +398,51 @@ public class DefaultObjectWrapper extends freemarker.ext.beans.BeansWrapper {
     }
 
     /**
+     * Getter pair of {@link #setDOMNodeSupport(boolean)}; see there.
+     *
+     * @since 2.3.31
+     */
+    public final boolean getDOMNodeSupport() {
+        return domNodeSupport;
+    }
+
+    /**
+     * Enables wrapping {@link Node}-s on a special way (as described in the "XML Processing Guide" in the Manual);
+     * defaults to {@code true}.. If this is {@code true}, {@link Node}+s will be wrapped like any other generic object.
+     *
+     * @see #handleUnknownType(Object)
+     *
+     * @since 2.3.31
+     */
+    public void setDOMNodeSupport(boolean domNodeSupport) {
+        checkModifiable();
+        this.domNodeSupport = domNodeSupport;
+    }
+
+    /**
+     * Getter pair of {@link #setJythonSupport(boolean)}; see there.
+     *
+     * @since 2.3.31
+     */
+    public final boolean getJythonSupport() {
+        return jythonSupport;
+    }
+
+    /**
+     * Enables wrapping Jython objects in a special way; defaults to {@code true}. If this is {@code false}, they will
+     * be wrapped like any other generic object. Note that Jython wrapping is legacy feature, and might by disabled by
+     * the selected {@link MemberAccessPolicy}, even if this is {@code true}; see {@link #handleUnknownType(Object)}.
+     *
+     * @see #handleUnknownType(Object)
+     *
+     * @since 2.3.31
+     */
+    public void setJythonSupport(boolean jythonSupport) {
+        checkModifiable();
+        this.jythonSupport = jythonSupport;
+    }
+
+    /**
      * Returns the lowest version number that is equivalent with the parameter version.
      * 
      * @since 2.3.22
@@ -416,8 +470,12 @@ public class DefaultObjectWrapper extends freemarker.ext.beans.BeansWrapper {
             }
         }
         
-        return "useAdaptersForContainers=" + useAdaptersForContainers + ", forceLegacyNonListCollections="
-                + forceLegacyNonListCollections + ", iterableSupport=" + iterableSupport + bwProps;
+        return "useAdaptersForContainers=" + useAdaptersForContainers
+                + ", forceLegacyNonListCollections=" + forceLegacyNonListCollections
+                + ", iterableSupport=" + iterableSupport
+                + ", domNodeSupport=" + domNodeSupport
+                + ", jythonSupport=" + jythonSupport
+                + bwProps;
     }
     
 }
diff --git a/src/main/java/freemarker/template/DefaultObjectWrapperConfiguration.java b/src/main/java/freemarker/template/DefaultObjectWrapperConfiguration.java
index ff474fa..a9575bc 100644
--- a/src/main/java/freemarker/template/DefaultObjectWrapperConfiguration.java
+++ b/src/main/java/freemarker/template/DefaultObjectWrapperConfiguration.java
@@ -35,6 +35,8 @@ public abstract class DefaultObjectWrapperConfiguration extends BeansWrapperConf
     private boolean useAdaptersForContainers;
     private boolean forceLegacyNonListCollections;
     private boolean iterableSupport;
+    private boolean domNodeSupport;
+    private boolean jythonSupport;
 
     protected DefaultObjectWrapperConfiguration(Version incompatibleImprovements) {
         super(DefaultObjectWrapper.normalizeIncompatibleImprovementsVersion(incompatibleImprovements), true);
@@ -43,6 +45,8 @@ public abstract class DefaultObjectWrapperConfiguration extends BeansWrapperConf
                 "freemarker.configuration", "DefaultObjectWrapper");
         useAdaptersForContainers = getIncompatibleImprovements().intValue() >= _TemplateAPI.VERSION_INT_2_3_22;
         forceLegacyNonListCollections = true; // [2.4]: = IcI < _TemplateAPI.VERSION_INT_2_4_0;
+        domNodeSupport = true;
+        jythonSupport = true;
     }
 
     /** See {@link DefaultObjectWrapper#getUseAdaptersForContainers()}. */
@@ -65,6 +69,26 @@ public abstract class DefaultObjectWrapperConfiguration extends BeansWrapperConf
         this.forceLegacyNonListCollections = legacyNonListCollectionWrapping;
     }
 
+    /** See {@link DefaultObjectWrapper#getDOMNodeSupport()}. */
+    public boolean getDOMNodeSupport() {
+        return domNodeSupport;
+    }
+
+    /** See {@link DefaultObjectWrapper#setDOMNodeSupport(boolean)}. */
+    public void setDOMNodeSupport(boolean domNodeSupport) {
+        this.domNodeSupport = domNodeSupport;
+    }
+
+    /** See {@link DefaultObjectWrapper#getJythonSupport()}. */
+    public boolean getJythonSupport() {
+        return jythonSupport;
+    }
+
+    /** See {@link DefaultObjectWrapper#setJythonSupport(boolean)}. */
+    public void setJythonSupport(boolean jythonSupport) {
+        this.jythonSupport = jythonSupport;
+    }
+
     /**
      * See {@link DefaultObjectWrapper#getIterableSupport()}.
      * 
@@ -90,6 +114,8 @@ public abstract class DefaultObjectWrapperConfiguration extends BeansWrapperConf
         result = result * prime + (useAdaptersForContainers ? 1231 : 1237);
         result = result * prime + (forceLegacyNonListCollections ? 1231 : 1237);
         result = result * prime + (iterableSupport ? 1231 : 1237);
+        result = result * prime + (domNodeSupport ? 1231 : 1237);
+        result = result * prime + (jythonSupport ? 1231 : 1237);
         return result;
     }
 
@@ -99,7 +125,9 @@ public abstract class DefaultObjectWrapperConfiguration extends BeansWrapperConf
         final DefaultObjectWrapperConfiguration thatDowCfg = (DefaultObjectWrapperConfiguration) that;
         return useAdaptersForContainers == thatDowCfg.getUseAdaptersForContainers()
                 && forceLegacyNonListCollections == thatDowCfg.forceLegacyNonListCollections
-                && iterableSupport == thatDowCfg.iterableSupport;
+                && iterableSupport == thatDowCfg.iterableSupport
+                && domNodeSupport == thatDowCfg.domNodeSupport
+                && jythonSupport == thatDowCfg.jythonSupport;
     }
 
 }
diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml
index a83ec5d..9d9123e 100644
--- a/src/manual/en_US/book.xml
+++ b/src/manual/en_US/book.xml
@@ -29179,18 +29179,18 @@ TemplateModel x = env.getVariable("x");  // get variable x</programlisting>
                 <para>If you are using the default object wrapper class
                 (<literal>freemarker.template.DefaultObjectWrapper</literal>),
                 or a subclass of it, you should disable the XML (DOM) wrapping
-                feature of it, by overriding <literal>wrapDomNode(Object
-                obj)</literal> so that it does this: <literal>return
-                getModelFactory(obj.getClass()).create(obj, this);</literal>.
-                The problem with the XML wrapping feature, which wraps
-                <literal>org.w3c.dom.Node</literal> objects on special way to
-                make them easier to work with in templates, is that this
-                facility by design lets template authors evaluate arbitrary
-                XPath expressions, and XPath can do too much in certain
-                setups. If you really need the XML wrapping facility, review
-                carefully what XPath expressions are possible in your setup.
-                Also, be sure you don't use the long deprecated, and more
-                dangerous <literal>freemarker.ext.xml</literal> package, only
+                feature of it, by setting its
+                <literal>DOMNodeSupport</literal> property to
+                <literal>false</literal>. The problem with the XML wrapping
+                feature, which wraps <literal>org.w3c.dom.Node</literal>
+                objects on special way to make them easier to work with in
+                templates, is that this facility by design lets template
+                authors evaluate arbitrary XPath expressions, and XPath can do
+                too much in certain setups. If you really need the XML
+                wrapping facility, review carefully what XPath expressions are
+                possible in your setup. Also, be sure you don't use the long
+                deprecated, and more dangerous
+                <literal>freemarker.ext.xml</literal> package, only
                 <literal>freemarker.ext.dom</literal>. Also, note that when
                 using the XML wrapping feature, not allowing
                 <literal>org.w3c.dom.Node</literal> methods in the
@@ -29419,6 +29419,16 @@ TemplateModel x = env.getVariable("x");  // get variable x</programlisting>
 
           <itemizedlist>
             <listitem>
+              <para>Added <literal>DOMNodeSupport</literal> and
+              <literal>JythonSupport</literal> <literal>boolean</literal>
+              properties to <literal>DefaultObjectWrapper</literal>. This
+              allows disabling the special wrapping of DOM nodes and Jython
+              classes. This might be desirable <link
+              linkend="faq_template_uploading_security">for security
+              reasons</link>.</para>
+            </listitem>
+
+            <listitem>
               <para><link
               xlink:href="https://issues.apache.org/jira/browse/FREEMARKER-145">FREEMARKER-145</link>:
               Fixed bug where methods with "overloaded" return type may become
diff --git a/src/test/java/freemarker/template/DefaultObjectWrapperTest.java b/src/test/java/freemarker/template/DefaultObjectWrapperTest.java
index b973358..d9924f1 100644
--- a/src/test/java/freemarker/template/DefaultObjectWrapperTest.java
+++ b/src/test/java/freemarker/template/DefaultObjectWrapperTest.java
@@ -46,7 +46,9 @@ import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
 
+import org.hamcrest.Matchers;
 import org.junit.Test;
+import org.python.core.PyString;
 import org.w3c.dom.Document;
 import org.xml.sax.InputSource;
 import org.xml.sax.SAXException;
@@ -58,6 +60,7 @@ import freemarker.ext.beans.BeansWrapper;
 import freemarker.ext.beans.EnumerationModel;
 import freemarker.ext.beans.HashAdapter;
 import freemarker.ext.beans.WhitelistMemberAccessPolicy;
+import freemarker.ext.jython.JythonSequenceModel;
 import freemarker.ext.util.WrapperTemplateModel;
 
 public class DefaultObjectWrapperTest {
@@ -1033,11 +1036,49 @@ public class DefaultObjectWrapperTest {
     @Test
     public void testCanWrapDOM() throws SAXException, IOException, ParserConfigurationException,
             TemplateModelException {
+        assertTrue(OW22.wrap(createDocument()) instanceof TemplateNodeModel);
+    }
+
+    @Test
+    public void testDisabledDOMNodeWrapping() throws SAXException, IOException, ParserConfigurationException,
+            TemplateModelException {
+        Document doc = createDocument();
+        DefaultObjectWrapperBuilder dowB = new DefaultObjectWrapperBuilder(Configuration.VERSION_2_3_31);
+        dowB.setDOMNodeSupport(false);
+        DefaultObjectWrapper ow = dowB.build();
+
+        testDisabledDomWrappingInternal(doc, ow);
+
+        ow = new DefaultObjectWrapper(Configuration.VERSION_2_3_31);
+        ow.setDOMNodeSupport(false);
+        testDisabledDomWrappingInternal(doc, ow);
+    }
+
+    private void testDisabledDomWrappingInternal(Document doc, DefaultObjectWrapper ow) throws TemplateModelException {
+        TemplateModel model = ow.wrap(doc);
+        assertFalse(model instanceof TemplateNodeModel);
+        assertTrue(model instanceof TemplateHashModel);
+        assertNotNull(((TemplateHashModel) model).get("getDoctype"));
+        assertNotNull(((TemplateHashModel) model).get("class"));
+    }
+
+    @Test
+    public void testDisabledJythonWrapping() throws SAXException, IOException, ParserConfigurationException,
+            TemplateModelException {
+        PyString pyString = new PyString("foo");
+
+        DefaultObjectWrapper ow = new DefaultObjectWrapper(Configuration.VERSION_2_3_31);
+        assertThat(ow.wrap(pyString), Matchers.instanceOf(JythonSequenceModel.class));
+
+        ow.setJythonSupport(false);
+        assertThat(ow.wrap(pyString), not(Matchers.instanceOf(JythonSequenceModel.class)));
+    }
+
+    private Document createDocument() throws ParserConfigurationException, SAXException, IOException {
         DocumentBuilder db = DocumentBuilderFactory.newInstance().newDocumentBuilder();
         InputSource is = new InputSource();
         is.setCharacterStream(new StringReader("<doc><sub a='1' /></doc>"));
-        Document doc = db.parse(is);        
-        assertTrue(OW22.wrap(doc) instanceof TemplateNodeModel);
+        return db.parse(is);
     }
 
     @Test