[2.3-next] Update Partial Rendering & Facelet path validation

impl/src/main/java/org/apache/myfaces/component/visit/PartialVisitContext.java

@@ -49,6 +49,14 @@
 public class PartialVisitContext extends VisitContext
 {
 
+    // Maximum NamingContainer nesting depth (number of separators) registered per client id.
+    // The number of separators in a client id equals its NamingContainer nesting depth; real views never
+    // nest more than a handful deep. Without a bound, a crafted client id made of many separators would make
+    // _addSubtreeClientId retain substring(0, i) for every separator, i.e. O(depth^2) characters and copies,
+    // which is an unauthenticated memory/CPU exhaustion vector (CWE-400). This keeps the work linear and acts
+    // as a backstop for any caller; the primary input caps live in PartialViewContextImpl.
+    private static final int MAX_NAMING_CONTAINER_DEPTH = 64;
+
     // The client ids to visit
     private final Collection<String> _clientIds;
 
@@ -331,7 +339,9 @@ private void _addSubtreeClientId(String clientId)
 
         int length = clientId.length();
 
-        for (int i = 0; i < length; i++)
+        // Bound the nesting depth we register to keep this method linear (see MAX_NAMING_CONTAINER_DEPTH).
+        int depth = 0;
+        for (int i = 0; i < length && depth < MAX_NAMING_CONTAINER_DEPTH; i++)
         {
             if (clientId.charAt(i) == separator)
             {
@@ -352,6 +362,8 @@ private void _addSubtreeClientId(String clientId)
 
                 // Stash away the client id
                 c.add(clientId);
+
+                depth++;
             }
         }
     }

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

@@ -89,7 +89,7 @@ else if (ex instanceof LocationAware)
 
         if (component != null)
         {
-            if (!location.isBlank())
+            if (!location.trim().isEmpty())
             {
                 location += ", ";
             }

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

@@ -0,0 +1,55 @@
+/*
+ * 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.context;
+
+import java.io.IOException;
+
+/** Exception thrown when a Facelet resource path fails security or mapping validation. */
+public class InvalidFileException extends IOException
+{
+    private static final long serialVersionUID = 1L;
+
+    /** Categorizes rejection reasons. */
+    public enum Reason
+    {
+        DISALLOWED_SCHEME,
+        PATH_TRAVERSAL,
+        INVALID_EXTENSION
+    }
+
+    private final Reason reason;
+
+    public InvalidFileException(Reason reason, String message)
+    {
+        super(message);
+        this.reason = reason;
+    }
+
+    public InvalidFileException(Reason reason, String message, Throwable cause)
+    {
+        super(message);
+        initCause(cause);
+        this.reason = reason;
+    }
+
+    public Reason getReason()
+    {
+        return reason;
+    }
+}

impl/src/main/java/org/apache/myfaces/context/servlet/PartialViewContextImpl.java

@@ -23,6 +23,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -69,8 +70,16 @@ public class PartialViewContextImpl extends PartialViewContext
     private static final String PARTIAL_AJAX = "partial/ajax";
     private static final String PARTIAL_AJAX_REQ = "javax.faces.partial.ajax";
     private static final String PARTIAL_PROCESS = "partial/process";
-
-    private static final Set<VisitHint> PARTIAL_EXECUTE_HINTS = Collections.unmodifiableSet( 
+
+    // Upper bounds for the attacker-controllable javax.faces.partial.render / .execute client id lists.
+    // A legitimate ajax request references only a handful of short client ids, so these caps never affect
+    // real traffic; they keep an unauthenticated caller from driving unbounded memory/CPU when the ids are
+    // expanded into a PartialVisitContext (CWE-400 quadratic resource exhaustion). See also the nesting-depth
+    // backstop in PartialVisitContext#_addSubtreeClientId.
+    private static final int MAX_CLIENT_IDS = 256;
+    private static final int MAX_CLIENT_ID_LENGTH = 256;
+
+    private static final Set<VisitHint> PARTIAL_EXECUTE_HINTS = Collections.unmodifiableSet(
             EnumSet.of(VisitHint.EXECUTE_LIFECYCLE, VisitHint.SKIP_UNRENDERED));
 
     private static final VisitCallback RESET_VALUES_CALLBACK = new ResetValuesCallback();
@@ -206,19 +215,9 @@ public Collection<String> getExecuteIds()
             if (executeMode != null && !executeMode.isEmpty()
                     && !PartialViewContext.ALL_PARTIAL_PHASE_CLIENT_IDS.equals(executeMode))
             {
-
-                String[] clientIds
-                        = StringUtils.splitShortString(_replaceTabOrEnterCharactersWithSpaces(executeMode), ' ');
-
                 //The collection must be mutable
-                List<String> tempList = new ArrayList<>(clientIds.length);
-                for (String clientId : clientIds)
-                {
-                    if (clientId.length() > 0)
-                    {
-                        tempList.add(clientId);
-                    }
-                }
+                Collection<String> tempList = parseClientIds(executeMode);
+
                 // The "javax.faces.source" parameter needs to be added to the list of
                 // execute ids if missing (otherwise, we'd never execute an action associated
                 // with, e.g., a button).
@@ -230,7 +229,9 @@ public Collection<String> getExecuteIds()
                 {
                     source = source.trim();
 
-                    if (!tempList.contains(source))
+                    // javax.faces.source is attacker-controlled as well; apply the same length bound so it
+                    // cannot bypass the cap and be expanded into an oversized PartialVisitContext (CWE-400).
+                    if (source.length() <= MAX_CLIENT_ID_LENGTH)
                     {
                         tempList.add(source);
                     }
@@ -277,6 +278,40 @@ private String _replaceTabOrEnterCharactersWithSpaces(String mode)
         return mode;
     }
 
+    /**
+     * Splits a space separated javax.faces.partial.render / .execute request parameter into its client ids.
+     * <p>
+     * The result is a mutable, insertion-ordered, duplicate-free collection. Empty tokens are dropped, client
+     * ids longer than {@link #MAX_CLIENT_ID_LENGTH} are rejected and at most {@link #MAX_CLIENT_IDS} ids are
+     * returned. These bounds keep an unauthenticated caller from expanding this attacker-controlled parameter
+     * into an oversized PartialVisitContext (CWE-400); legitimate requests stay well below the limits.
+     */
+    private Collection<String> parseClientIds(String mode)
+    {
+        String[] clientIds = StringUtils.splitShortString(_replaceTabOrEnterCharactersWithSpaces(mode), ' ');
+
+        // LinkedHashSet: collapse duplicate client ids once, here, instead of carrying them through the
+        // request, while preserving order.
+        Collection<String> result = new LinkedHashSet<>();
+        for (String clientId : clientIds)
+        {
+            int length = clientId.length();
+            if (length == 0 || length > MAX_CLIENT_ID_LENGTH)
+            {
+                // skip empty tokens and reject implausibly long client ids
+                continue;
+            }
+
+            result.add(clientId);
+
+            if (result.size() >= MAX_CLIENT_IDS)
+            {
+                break;
+            }
+        }
+        return result;
+    }
+
     @Override
     public Collection<String> getRenderIds()
     {
@@ -291,19 +326,8 @@ public Collection<String> getRenderIds()
             if (renderMode != null && !renderMode.isEmpty()
                     && !PartialViewContext.ALL_PARTIAL_PHASE_CLIENT_IDS.equals(renderMode))
             {
-                String[] clientIds
-                        = StringUtils.splitShortString(_replaceTabOrEnterCharactersWithSpaces(renderMode), ' ');
-
                 //The collection must be mutable
-                List<String> tempList = new ArrayList<>(clientIds.length);
-                for (String clientId : clientIds)
-                {
-                    if (clientId.length() > 0)
-                    {
-                        tempList.add(clientId);
-                    }
-                }
-                _renderClientIds = tempList;
+                _renderClientIds = parseClientIds(renderMode);
             }
             else
             {