MYFACES-4266: Ajax update fails due to invalid characters in response XML (DoS)

impl/src/main/java/org/apache/myfaces/context/PartialResponseWriterImpl.java

@@ -29,6 +29,7 @@
 import javax.faces.context.ResponseWriter;
 
 import org.apache.myfaces.util.CDataEndEscapeFilterWriter;
+import org.apache.myfaces.util.IllegalXmlCharacterFilterWriter;
 
 /**
  * <p>
@@ -110,7 +111,7 @@ public void setDoubleBuffer(Writer doubleBuffer)
 
     public PartialResponseWriterImpl(ResponseWriter writer)
     {
-        super(writer);
+        super(writer.cloneWithWriter(new IllegalXmlCharacterFilterWriter(writer)));
     }
 
     @Override
@@ -129,7 +130,7 @@ public void startCDATA() throws IOException
 
     private void openDoubleBuffer()
     {
-        _doubleBuffer = new CDataEndEscapeFilterWriter(_cdataDoubleBufferWriter == null ? 
+        _doubleBuffer = new CDataEndEscapeFilterWriter(_cdataDoubleBufferWriter == null ?
                 this.getWrapped() : _cdataDoubleBufferWriter );
         _cdataDoubleBufferWriter = getWrapped().cloneWithWriter(_doubleBuffer);
 

impl/src/main/java/org/apache/myfaces/util/IllegalXmlCharacterFilterWriter.java

@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.myfaces.util;
+
+import java.io.FilterWriter;
+import java.io.IOException;
+import java.io.Writer;
+
+/**
+ * There are unicodes outside the ranges defined in the <a href="https://www.w3.org/TR/REC-xml/#charsets">XML 1.0 specification</a> that break XML parsers
+ * and therefore must be filtered out when writing partial responses. Otherwise this may lead to Denial of Service attacks.
+ * @see https://issues.apache.org/jira/browse/MYFACES-4266
+ */
+public class IllegalXmlCharacterFilterWriter extends FilterWriter
+{
+    public IllegalXmlCharacterFilterWriter(Writer out)
+    {
+        super(out);
+    }
+
+    @Override
+    public void write(int c) throws IOException 
+    {
+        super.write(xmlEncode((char) c));
+    }
+
+    @Override
+    public void write(char[] cbuf, int off, int len) throws IOException 
+    {
+        super.write(xmlEncode(cbuf), off, len);
+    }
+
+    @Override
+    public void write(String str, int off, int len) throws IOException 
+    {
+        super.write(new String(xmlEncode(str.toCharArray())), off, len);
+    }
+
+    private char[] xmlEncode(char[] ca)
+    {
+        for (int i = 0; i < ca.length; i++)
+        {
+            ca[i] = xmlEncode(ca[i]);
+        }
+        return ca;
+    }
+
+    private char xmlEncode(char c)
+    {
+        if (Character.isSurrogate(c)) 
+        {
+            return ' ';
+        }
+        if (c == '\u0009' || c == '\n' || c == '\r') 
+        {
+            return c;
+        }
+        if (c > '\u0020' && c < '\uD7FF') 
+        {
+            return c;
+        }
+        if (c > '\uE000' && c < '\uFFFD') 
+        {
+            return c;
+        }
+        return ' ';
+    }
+}

impl/src/test/java/org/apache/myfaces/context/PartialResponseWriterImplTest.java

@@ -286,8 +286,6 @@ public void testBrokenUserInput() {
         }
     }
 
-
-
     public void testDelete() {
         _writer = createTestProbe();
         try {
@@ -298,8 +296,48 @@ public void testDelete() {
         }
     }
 
+    public void testWriteIllegalXmlUnicodeCharacters() {
+        _writer = createTestProbe();
+        try {
+            String illegalChars = " \u0001\u0002\u0003\u0004\u0005\u0006\u000B\f\u000E\u000F\u0010\u0011\u0012\u0013\u0014\u0015\u0016\u0017\u0018\u0019\u001A\u001B\u001C\u001D\u001E\u001F \uD7FF\uDBFF\uDC00\uE000��";
+            String legalChars = "foo";
+            _writer.write(illegalChars + legalChars);
+            assertEquals("All illegal XML unicode characters should have been replaced by spaces", legalChars, _contentCollector.toString().trim());
+
+        } catch (IOException e) {
+            fail(e.toString());
+        }
+    }
+
+    public void testWriteTextIllegalXmlUnicodeCharacters() {
+        _writer = createTestProbe();
+        try {
+            String illegalChars = " \u0001\u0002\u0003\u0004\u0005\u0006\u000B\f\u000E\u000F\u0010\u0011\u0012\u0013\u0014\u0015\u0016\u0017\u0018\u0019\u001A\u001B\u001C\u001D\u001E\u001F \uD7FF\uDBFF\uDC00\uE000��";
+            String legalChars = "foo";
+            _writer.writeText(illegalChars + legalChars, null);
+            assertEquals("All illegal XML unicode characters should have been replaced by spaces", legalChars, _contentCollector.toString().trim());
+
+        } catch (IOException e) {
+            fail(e.toString());
+        }
+    }
 
+    public void testWriteAttributeIllegalXmlUnicodeCharacters() {
+        _writer = createTestProbe();
+        try {
+            String illegalChars = " \u0001\u0002\u0003\u0004\u0005\u0006\u000B\f\u000E\u000F\u0010\u0011\u0012\u0013\u0014\u0015\u0016\u0017\u0018\u0019\u001A\u001B\u001C\u001D\u001E\u001F \uD7FF\uDBFF\uDC00\uE000��";
+            String legalChars = "foo";
+            _writer.startElement(legalChars, null);
+            _writer.writeAttribute(legalChars, illegalChars + legalChars, null);
+            _writer.endElement(legalChars);
+            assertTrue("All illegal XML unicode characters should have been replaced by spaces", 
+                    _contentCollector.toString().matches("<:X: :X:=\"[ ]+:X:\"></:X:>".replace(":X:", legalChars)));
 
+        } catch (IOException e) {
+            fail(e.toString());
+        }
+    }
+
     /**
      * creates a new test probe (aka response writer)
      *