From 5a793f7400217ab6524716a9f8b99d6a493045d7 Mon Sep 17 00:00:00 2001 From: Volodymyr Siedlecki Date: Wed, 12 Aug 2026 23:03:43 -0400 Subject: [PATCH 1/6] [bug fix] Add resource path validation to DefaultFaceletFactory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Strengthen resolveURL() with four layered guards to prevent unsafe Facelet resource resolution: 1. Scheme check — rejects remote/network URI schemes (http, https, ftp, etc.) using a fast colon-index scan; OSGi/container schemes (jar, wsjar, file, zip) are intentionally allowed. 2. Traversal guard — for relative paths, verifies the resolved URL remains within the application base (WAR/EAR root) to prevent directory traversal attacks. 3. WEB-INF XML guard (scaffolded, currently disabled) — isWebInfConfigFile() is in place to block XML config descriptors under WEB-INF/ if .xml is ever added as a Facelet suffix. 4. Extension/suffix check — rejects paths whose extension is not in the configured Facelet suffix set (jakarta.faces.FACELETS_SUFFIX / jakarta.faces.FACELETS_VIEW_MAPPINGS), cached after the first call. Also adds FINE-level logging at each rejection point and a log of the computed allowed-suffix set on first initialisation. UnitTest project stage bypasses guards 2-4 to allow tests that use synthetic paths not backed by a real WAR layout. AI Assisted: Bob Version: 2.0.2 --- .../facelets/impl/DefaultFaceletFactory.java | 260 +++++++++++++++++- 1 file changed, 255 insertions(+), 5 deletions(-) diff --git a/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java b/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java index 2b18271d3..e340e3a7c 100644 --- a/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java +++ b/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java @@ -21,9 +21,12 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.net.URL; +import java.util.Arrays; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; @@ -32,8 +35,12 @@ import jakarta.faces.FacesException; import jakarta.faces.FactoryFinder; import jakarta.faces.annotation.View; +import jakarta.faces.application.ProjectStage; +import jakarta.faces.application.ViewHandler; import jakarta.faces.application.ViewResource; +import jakarta.faces.context.ExternalContext; import jakarta.faces.context.FacesContext; +import java.net.MalformedURLException; import jakarta.faces.view.facelets.Facelet; import jakarta.faces.view.facelets.FaceletCache; import jakarta.faces.view.facelets.FaceletCacheFactory; @@ -67,6 +74,7 @@ public final class DefaultFaceletFactory extends FaceletFactory private long _refreshPeriod; private Map _relativeLocations; private Map _managedFacelet; + private volatile Set _allowedSuffixes; private FaceletCache _faceletCache; private AbstractFaceletCache _abstractFaceletCache; @@ -263,20 +271,262 @@ public long getRefreshPeriod() */ public URL resolveURL(FacesContext context, URL source, String path) throws IOException { - if (path.startsWith("/")) + // --- 1. Reject remote/network schemes (http, ftp, etc.) up front. + // OSGi/container schemes (wsjar, jar, file, zip) are allowed and pass through. + if (!isAllowedScheme(path)) { + throw new MalformedURLException( + "Remote or disallowed scheme in path: " + path); + } + + URL resolved; + String normalizedPath; + boolean absoluteContextPath = path.startsWith("/"); + + if (absoluteContextPath) + { + // Absolute context-relative path — resolved through ExternalContext. + // The container already scopes the lookup to the WAR, so no traversal is possible. context.getAttributes().put(LAST_RESOURCE_RESOLVED, null); - URL url = resolveURL(context, path); - if (url == null) + resolved = resolveURL(context, path); + if (resolved == null) { throw new FileNotFoundException(path + " Not Found in ExternalContext as a Resource"); } - return url; + normalizedPath = path; } else { - return new URL(source, path); + // Relative path — resolved against the current source URL. + resolved = new URL(source, path); + normalizedPath = resolved.getPath(); + } + + // UnitTest stage skips content-validation guards; tests use synthetic paths + // that are not backed by a real WAR layout. + if (context.isProjectStage(ProjectStage.UnitTest)) + { + return resolved; + } + + // --- 2. File must be inside the WAR/EAR (traversal guard for relative paths only). + // Absolute context paths are already scoped by ExternalContext. + if (!absoluteContextPath && !isWithinBase(resolved)) + { + if (log.isLoggable(Level.FINE)) + { + log.fine("Path not allowed [" + path + "] -> resolved URL escapes application base"); + } + throw new MalformedURLException( + "Path escapes application base: " + path); + } + + // --- 3. WEB-INF XML config files must not be directly served as Facelets. + // check mark is disabled + // Reason: .xml is not a facelet file unless specified via suffix / mapping parameters + // if (isWebInfConfigFile(normalizedPath)) + // { + // if (log.isLoggable(Level.FINE)) + // { + // log.fine("Path not allowed [" + path + "] -> WEB-INF XML config file"); + // } + // throw new MalformedURLException("Access to WEB-INF XML config files is not allowed: " + path); + // } + + // --- 4. Extension must be a configured Facelet suffix (e.g. .xhtml, .jspx). + if (!mappingAllowed(context, normalizedPath)) + { + if (log.isLoggable(Level.FINE)) + { + log.fine("Path not allowed [" + path + "] -> extension not a configured Facelet suffix"); + } + throw new MalformedURLException( + "Invalid path provided: " + path); + } + + return resolved; + } + + // --------------------------------------------------------------------------- + // Path-validation helpers + // --------------------------------------------------------------------------- + + /** + * Remote/network URI schemes that must never be used as Facelet resource paths. + * OSGi container schemes (wsjar, jar, file, zip) are intentionally absent — + * they reference in-archive resources and are therefore safe. + */ + private static final Set BLOCKED_SCHEMES = new HashSet<>( + Arrays.asList("http", "https", "ftp", "ftps", "mailto", "tel", + "imap", "irc", "nntp", "acap", "icap", "mtqp", "wss")); + + /** + * Returns {@code false} when {@code path} is an absolute URI whose scheme is on the + * {@link #BLOCKED_SCHEMES} list. Purely relative paths (no scheme) and OSGi/container + * schemes (wsjar, jar, file, zip) always return {@code true}. + *

+ * Uses a fast colon-index pre-check to avoid {@code URI} allocation for the common case + * of relative or context-root paths (e.g. {@code /views/page.xhtml}). + */ + private boolean isAllowedScheme(String path) + { + // Fast path: a scheme requires at least one letter before ":", so the colon must + // appear at index >= 1. Relative paths and "/"-absolute paths never have a colon + // in this position and are immediately allowed. + int colon = path.indexOf(':'); + if (colon < 1) + { + return true; + } + String scheme = path.substring(0, colon).toLowerCase(); + if (BLOCKED_SCHEMES.contains(scheme)) + { + if (log.isLoggable(Level.FINE)) + { + log.fine("Path not allowed [" + path + "] -> Blocked scheme: " + scheme); + } + return false; + } + return true; + } + + /** + * Returns {@code true} when {@code normalizedPath} refers to an XML file located under + * {@code /WEB-INF/}. Such files are server configuration descriptors and must never be + * exposed as Facelet templates. + */ + private boolean isWebInfConfigFile(String normalizedPath) + { + if (normalizedPath == null) + { + return false; + } + String lower = normalizedPath.replace('\\', '/').toLowerCase(); + return lower.contains("/web-inf/") && lower.endsWith(".xml"); + } + + /** + * Verifies that {@code resolved} is contained within the application base URL + * (i.e. the WAR/EAR root), preventing directory traversal outside the archive. + */ + private boolean isWithinBase(URL resolved) + { + URL base = getBaseUrl(); + if (base == null) + { + return true; // cannot determine base — allow and let the container decide + } + String baseStr = base.toExternalForm(); + String resolvedStr = resolved.toExternalForm(); + if (!baseStr.endsWith("/")) + { + baseStr = baseStr + "/"; + } + // For jar:/wsjar: URLs the in-archive path follows "!/"; the shared jar + // file prefix is enough to confirm containment. + return resolvedStr.startsWith(baseStr); + } + + /** + * Returns {@code true} when {@code normalizedPath} ends with a suffix that is configured + * as an allowed Facelet extension. Built from {@code jakarta.faces.FACELETS_SUFFIX} + * (default {@code .xhtml}) and suffix entries in {@code jakarta.faces.FACELETS_VIEW_MAPPINGS}. + * {@code .jspx} is always included for legacy JSP-XML views. + *

+ * Note: {@code .xml} is intentionally not added here; XML files under + * {@code WEB-INF/} are blocked by {@link #isWebInfXml(String)} and plain {@code .xml} + * outside that directory is not a valid Facelet extension. + */ + private boolean mappingAllowed(FacesContext context, String normalizedPath) + { + if (normalizedPath == null || normalizedPath.isEmpty()) + { + if (log.isLoggable(Level.FINE)) + { + log.fine("Mapping not allowed [" + normalizedPath + "] -> Empty or null path"); + } + return false; + } + int dotIndex = normalizedPath.lastIndexOf('.'); + if (dotIndex < 0) + { + if (log.isLoggable(Level.FINE)) + { + log.fine("Mapping not allowed [" + normalizedPath + "] -> No extension"); + } + return false; + } + String ext = normalizedPath.substring(dotIndex); + + if (!getAllowedSuffixes(context).contains(ext)) + { + if (log.isLoggable(Level.FINE)) + { + log.fine("Mapping not allowed [" + normalizedPath + "] -> Extension not a Facelet suffix: " + ext); + } + return false; + } + return true; + } + + /** + * Returns the set of allowed Facelet file suffixes, computed once from the application's + * init parameters and cached for the lifetime of this factory. + *

+ * The set is built from: + *

    + *
  • {@code jakarta.faces.FACELETS_SUFFIX} (whitespace-separated, default {@code .xhtml})
    • + *
  • Suffix entries in {@code jakarta.faces.FACELETS_VIEW_MAPPINGS} (semicolon-separated; + * entries beginning with {@code *.} contribute the extension part)
    • + *
  • {@code .jspx} — always included for legacy JSP-XML views
    • + *
+ * Init parameters are read only on the first call; subsequent calls return the cached set. + */ + private Set getAllowedSuffixes(FacesContext context) + { + if (_allowedSuffixes == null) + { + ExternalContext ec = context.getExternalContext(); + + // Suffixes from jakarta.faces.FACELETS_SUFFIX (whitespace-separated, default ".xhtml") + String suffixParam = ec.getInitParameter(ViewHandler.FACELETS_SUFFIX_PARAM_NAME); + if (suffixParam == null) + { + suffixParam = ViewHandler.DEFAULT_FACELETS_SUFFIX; + } + Set allowed = new HashSet<>(Arrays.asList(suffixParam.trim().split("\\s+"))); + + // Suffixes from jakarta.faces.FACELETS_VIEW_MAPPINGS (semicolon-separated; strip leading "*") + String mappingsParam = ec.getInitParameter(ViewHandler.FACELETS_VIEW_MAPPINGS_PARAM_NAME); + if (mappingsParam == null) + { + mappingsParam = ec.getInitParameter("facelets.VIEW_MAPPINGS"); + } + if (mappingsParam != null) + { + for (String token : mappingsParam.split(";")) + { + token = token.trim(); + if (token.startsWith("*.")) + { + // suffix mapping e.g. "*.xhtml" -> ".xhtml" + allowed.add(token.substring(1)); + } + // Prefix mappings like "/faces/*" carry no extension — skipped. + } + } + + // Legacy JSP-XML view support + // allowed.add(".jspx"); + + if (log.isLoggable(Level.FINE)) + { + log.fine("Allowed Facelet suffixes: " + allowed); + } + + _allowedSuffixes = allowed; } + return _allowedSuffixes; } /** From 5f2b6be79c9519bbee735c8fe5b4d8e9a5ca5307 Mon Sep 17 00:00:00 2001 From: Volodymyr Siedlecki Date: Wed, 12 Aug 2026 23:04:26 -0400 Subject: [PATCH 2/6] [refactor] Introduce InvalidFileException for Facelet path rejections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add InvalidFileException (extends IOException) with a typed Reason enum to replace the generic MalformedURLException throws in resolveURL(). Reason values: DISALLOWED_SCHEME — blocked remote/network URI scheme PATH_TRAVERSAL — resolved URL escapes the application base INVALID_EXTENSION — extension not a configured Facelet suffix Callers can now catch InvalidFileException and inspect getReason() to programmatically distinguish between rejection causes without parsing exception messages. AI Assisted: Bob Version: 2.0.2 --- .../myfaces/context/InvalidFileException.java | 92 +++++++++++++++++++ .../facelets/impl/DefaultFaceletFactory.java | 8 +- 2 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 impl/src/main/java/org/apache/myfaces/context/InvalidFileException.java diff --git a/impl/src/main/java/org/apache/myfaces/context/InvalidFileException.java b/impl/src/main/java/org/apache/myfaces/context/InvalidFileException.java new file mode 100644 index 000000000..905bedba1 --- /dev/null +++ b/impl/src/main/java/org/apache/myfaces/context/InvalidFileException.java @@ -0,0 +1,92 @@ +/* + * 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; + +/** + * Thrown when a Facelet resource path is rejected by the security or mapping + * validation checks in the Facelet factory. + *

+ * The {@link Reason} enum identifies which specific check triggered the rejection, + * allowing callers to distinguish between a blocked URI scheme, a path that escapes + * the application base, a protected WEB-INF config file, and an extension that is + * not a configured Facelet suffix. + */ +public class InvalidFileException extends IOException +{ + private static final long serialVersionUID = 1L; + + /** + * Categorises why a resource path was rejected. + */ + public enum Reason + { + /** The path contains a remote or otherwise disallowed URI scheme (e.g. {@code http:}, {@code ftp:}). */ + DISALLOWED_SCHEME, + + /** The resolved URL escapes the application's WAR/EAR base directory (path-traversal attempt). */ + PATH_TRAVERSAL, + + /** The path targets an XML configuration file under {@code WEB-INF/} (e.g. {@code web.xml}). */ + // WEBINF_CONFIG_FILE, // Likely not needed? + + /** The file extension is not among the configured Facelet suffixes. */ + INVALID_EXTENSION + } + + private final Reason reason; + + /** + * Constructs an {@code InvalidFileException} with the given rejection reason and detail message. + * + * @param reason the specific cause of the rejection; must not be {@code null} + * @param message a human-readable description of the rejected path and why it was blocked + */ + public InvalidFileException(Reason reason, String message) + { + super(message); + this.reason = reason; + } + + /** + * Constructs an {@code InvalidFileException} with the given rejection reason, detail message, + * and underlying cause. + * + * @param reason the specific cause of the rejection; must not be {@code null} + * @param message a human-readable description of the rejected path and why it was blocked + * @param cause the original exception that triggered this rejection, or {@code null} + */ + public InvalidFileException(Reason reason, String message, Throwable cause) + { + super(message); + initCause(cause); + this.reason = reason; + } + + /** + * Returns the reason this file path was considered invalid. + * + * @return the rejection {@link Reason}; never {@code null} + */ + public Reason getReason() + { + return reason; + } +} diff --git a/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java b/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java index e340e3a7c..91e98b89c 100644 --- a/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java +++ b/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java @@ -40,7 +40,7 @@ import jakarta.faces.application.ViewResource; import jakarta.faces.context.ExternalContext; import jakarta.faces.context.FacesContext; -import java.net.MalformedURLException; +import org.apache.myfaces.context.InvalidFileException; import jakarta.faces.view.facelets.Facelet; import jakarta.faces.view.facelets.FaceletCache; import jakarta.faces.view.facelets.FaceletCacheFactory; @@ -275,7 +275,7 @@ public URL resolveURL(FacesContext context, URL source, String path) throws IOEx // OSGi/container schemes (wsjar, jar, file, zip) are allowed and pass through. if (!isAllowedScheme(path)) { - throw new MalformedURLException( + throw new InvalidFileException(InvalidFileException.Reason.DISALLOWED_SCHEME, "Remote or disallowed scheme in path: " + path); } @@ -317,7 +317,7 @@ public URL resolveURL(FacesContext context, URL source, String path) throws IOEx { log.fine("Path not allowed [" + path + "] -> resolved URL escapes application base"); } - throw new MalformedURLException( + throw new InvalidFileException(InvalidFileException.Reason.PATH_TRAVERSAL, "Path escapes application base: " + path); } @@ -340,7 +340,7 @@ public URL resolveURL(FacesContext context, URL source, String path) throws IOEx { log.fine("Path not allowed [" + path + "] -> extension not a configured Facelet suffix"); } - throw new MalformedURLException( + throw new InvalidFileException(InvalidFileException.Reason.INVALID_EXTENSION, "Invalid path provided: " + path); } From ff0c96885c9081ec1d7f90542c9ca6a51e3ae5da Mon Sep 17 00:00:00 2001 From: Volodymyr Siedlecki Date: Thu, 13 Aug 2026 09:56:55 -0400 Subject: [PATCH 3/6] [code review] Reduce comments, including web-inf commented out code --- .../myfaces/context/InvalidFileException.java | 41 +----- .../facelets/impl/DefaultFaceletFactory.java | 130 +++--------------- 2 files changed, 20 insertions(+), 151 deletions(-) diff --git a/impl/src/main/java/org/apache/myfaces/context/InvalidFileException.java b/impl/src/main/java/org/apache/myfaces/context/InvalidFileException.java index 905bedba1..7a15ac53a 100644 --- a/impl/src/main/java/org/apache/myfaces/context/InvalidFileException.java +++ b/impl/src/main/java/org/apache/myfaces/context/InvalidFileException.java @@ -20,59 +20,27 @@ import java.io.IOException; -/** - * Thrown when a Facelet resource path is rejected by the security or mapping - * validation checks in the Facelet factory. - *

- * The {@link Reason} enum identifies which specific check triggered the rejection, - * allowing callers to distinguish between a blocked URI scheme, a path that escapes - * the application base, a protected WEB-INF config file, and an extension that is - * not a configured Facelet suffix. - */ +/** Exception thrown when a Facelet resource path fails security or mapping validation. */ public class InvalidFileException extends IOException { private static final long serialVersionUID = 1L; - /** - * Categorises why a resource path was rejected. - */ + /** Categorizes rejection reasons. */ public enum Reason { - /** The path contains a remote or otherwise disallowed URI scheme (e.g. {@code http:}, {@code ftp:}). */ DISALLOWED_SCHEME, - - /** The resolved URL escapes the application's WAR/EAR base directory (path-traversal attempt). */ PATH_TRAVERSAL, - - /** The path targets an XML configuration file under {@code WEB-INF/} (e.g. {@code web.xml}). */ - // WEBINF_CONFIG_FILE, // Likely not needed? - - /** The file extension is not among the configured Facelet suffixes. */ INVALID_EXTENSION } private final Reason reason; - /** - * Constructs an {@code InvalidFileException} with the given rejection reason and detail message. - * - * @param reason the specific cause of the rejection; must not be {@code null} - * @param message a human-readable description of the rejected path and why it was blocked - */ public InvalidFileException(Reason reason, String message) { super(message); this.reason = reason; } - /** - * Constructs an {@code InvalidFileException} with the given rejection reason, detail message, - * and underlying cause. - * - * @param reason the specific cause of the rejection; must not be {@code null} - * @param message a human-readable description of the rejected path and why it was blocked - * @param cause the original exception that triggered this rejection, or {@code null} - */ public InvalidFileException(Reason reason, String message, Throwable cause) { super(message); @@ -80,11 +48,6 @@ public InvalidFileException(Reason reason, String message, Throwable cause) this.reason = reason; } - /** - * Returns the reason this file path was considered invalid. - * - * @return the rejection {@link Reason}; never {@code null} - */ public Reason getReason() { return reason; diff --git a/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java b/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java index 91e98b89c..f3944618d 100644 --- a/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java +++ b/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java @@ -257,22 +257,17 @@ public long getRefreshPeriod() } /** - * Resolves a path based on the passed URL. If the path starts with '/', then resolve the path against - * {@link jakarta.faces.context.ExternalContext#getResource(java.lang.String) - * jakarta.faces.context.ExternalContext#getResource(java.lang.String)}. Otherwise create a new URL via - * {@link URL#URL(java.net.URL, java.lang.String) URL(URL, String)}. - * - * @param source - * base to resolve from - * @param path - * relative path to the source + * Resolves a path to a URL, validating scheme, traversal, and extension. + * Absolute paths (starting with '/') are resolved via ExternalContext; + * relative paths are resolved against the source URL. + * @param context FacesContext + * @param source base URL for relative resolution + * @param path path to resolve * @return resolved URL - * @throws IOException + * @throws IOException if path is invalid or not found */ public URL resolveURL(FacesContext context, URL source, String path) throws IOException { - // --- 1. Reject remote/network schemes (http, ftp, etc.) up front. - // OSGi/container schemes (wsjar, jar, file, zip) are allowed and pass through. if (!isAllowedScheme(path)) { throw new InvalidFileException(InvalidFileException.Reason.DISALLOWED_SCHEME, @@ -285,8 +280,7 @@ public URL resolveURL(FacesContext context, URL source, String path) throws IOEx if (absoluteContextPath) { - // Absolute context-relative path — resolved through ExternalContext. - // The container already scopes the lookup to the WAR, so no traversal is possible. + // Absolute context-relative path via ExternalContext (scoped to WAR by container) context.getAttributes().put(LAST_RESOURCE_RESOLVED, null); resolved = resolveURL(context, path); if (resolved == null) @@ -297,49 +291,27 @@ public URL resolveURL(FacesContext context, URL source, String path) throws IOEx } else { - // Relative path — resolved against the current source URL. + // Relative path resolved against source URL resolved = new URL(source, path); normalizedPath = resolved.getPath(); } - // UnitTest stage skips content-validation guards; tests use synthetic paths - // that are not backed by a real WAR layout. + // Skip validation in UnitTest stage (uses synthetic paths) if (context.isProjectStage(ProjectStage.UnitTest)) { return resolved; } - // --- 2. File must be inside the WAR/EAR (traversal guard for relative paths only). - // Absolute context paths are already scoped by ExternalContext. + // Traversal guard: relative paths must stay within base (absolute paths already scoped by container) if (!absoluteContextPath && !isWithinBase(resolved)) { - if (log.isLoggable(Level.FINE)) - { - log.fine("Path not allowed [" + path + "] -> resolved URL escapes application base"); - } throw new InvalidFileException(InvalidFileException.Reason.PATH_TRAVERSAL, "Path escapes application base: " + path); } - // --- 3. WEB-INF XML config files must not be directly served as Facelets. - // check mark is disabled - // Reason: .xml is not a facelet file unless specified via suffix / mapping parameters - // if (isWebInfConfigFile(normalizedPath)) - // { - // if (log.isLoggable(Level.FINE)) - // { - // log.fine("Path not allowed [" + path + "] -> WEB-INF XML config file"); - // } - // throw new MalformedURLException("Access to WEB-INF XML config files is not allowed: " + path); - // } - - // --- 4. Extension must be a configured Facelet suffix (e.g. .xhtml, .jspx). + // Extension must be a configured Facelet suffix if (!mappingAllowed(context, normalizedPath)) { - if (log.isLoggable(Level.FINE)) - { - log.fine("Path not allowed [" + path + "] -> extension not a configured Facelet suffix"); - } throw new InvalidFileException(InvalidFileException.Reason.INVALID_EXTENSION, "Invalid path provided: " + path); } @@ -347,32 +319,15 @@ public URL resolveURL(FacesContext context, URL source, String path) throws IOEx return resolved; } - // --------------------------------------------------------------------------- // Path-validation helpers - // --------------------------------------------------------------------------- - /** - * Remote/network URI schemes that must never be used as Facelet resource paths. - * OSGi container schemes (wsjar, jar, file, zip) are intentionally absent — - * they reference in-archive resources and are therefore safe. - */ private static final Set BLOCKED_SCHEMES = new HashSet<>( Arrays.asList("http", "https", "ftp", "ftps", "mailto", "tel", "imap", "irc", "nntp", "acap", "icap", "mtqp", "wss")); - /** - * Returns {@code false} when {@code path} is an absolute URI whose scheme is on the - * {@link #BLOCKED_SCHEMES} list. Purely relative paths (no scheme) and OSGi/container - * schemes (wsjar, jar, file, zip) always return {@code true}. - *

- * Uses a fast colon-index pre-check to avoid {@code URI} allocation for the common case - * of relative or context-root paths (e.g. {@code /views/page.xhtml}). - */ + /** Returns false if path has a blocked scheme; true for relative/container schemes. */ private boolean isAllowedScheme(String path) { - // Fast path: a scheme requires at least one letter before ":", so the colon must - // appear at index >= 1. Relative paths and "/"-absolute paths never have a colon - // in this position and are immediately allowed. int colon = path.indexOf(':'); if (colon < 1) { @@ -390,11 +345,7 @@ private boolean isAllowedScheme(String path) return true; } - /** - * Returns {@code true} when {@code normalizedPath} refers to an XML file located under - * {@code /WEB-INF/}. Such files are server configuration descriptors and must never be - * exposed as Facelet templates. - */ + /** Returns true if normalizedPath refers to a WEB-INF XML config file. */ private boolean isWebInfConfigFile(String normalizedPath) { if (normalizedPath == null) @@ -405,16 +356,13 @@ private boolean isWebInfConfigFile(String normalizedPath) return lower.contains("/web-inf/") && lower.endsWith(".xml"); } - /** - * Verifies that {@code resolved} is contained within the application base URL - * (i.e. the WAR/EAR root), preventing directory traversal outside the archive. - */ + /** Verifies that resolved URL is contained within the application base. */ private boolean isWithinBase(URL resolved) { URL base = getBaseUrl(); if (base == null) { - return true; // cannot determine base — allow and let the container decide + return true; } String baseStr = base.toExternalForm(); String resolvedStr = resolved.toExternalForm(); @@ -422,73 +370,37 @@ private boolean isWithinBase(URL resolved) { baseStr = baseStr + "/"; } - // For jar:/wsjar: URLs the in-archive path follows "!/"; the shared jar - // file prefix is enough to confirm containment. return resolvedStr.startsWith(baseStr); } - /** - * Returns {@code true} when {@code normalizedPath} ends with a suffix that is configured - * as an allowed Facelet extension. Built from {@code jakarta.faces.FACELETS_SUFFIX} - * (default {@code .xhtml}) and suffix entries in {@code jakarta.faces.FACELETS_VIEW_MAPPINGS}. - * {@code .jspx} is always included for legacy JSP-XML views. - *

- * Note: {@code .xml} is intentionally not added here; XML files under - * {@code WEB-INF/} are blocked by {@link #isWebInfXml(String)} and plain {@code .xml} - * outside that directory is not a valid Facelet extension. - */ + /** Returns true if normalizedPath ends with a configured Facelet extension. */ private boolean mappingAllowed(FacesContext context, String normalizedPath) { if (normalizedPath == null || normalizedPath.isEmpty()) { - if (log.isLoggable(Level.FINE)) - { - log.fine("Mapping not allowed [" + normalizedPath + "] -> Empty or null path"); - } return false; } int dotIndex = normalizedPath.lastIndexOf('.'); if (dotIndex < 0) { - if (log.isLoggable(Level.FINE)) - { - log.fine("Mapping not allowed [" + normalizedPath + "] -> No extension"); - } return false; } String ext = normalizedPath.substring(dotIndex); if (!getAllowedSuffixes(context).contains(ext)) { - if (log.isLoggable(Level.FINE)) - { - log.fine("Mapping not allowed [" + normalizedPath + "] -> Extension not a Facelet suffix: " + ext); - } return false; } return true; } - /** - * Returns the set of allowed Facelet file suffixes, computed once from the application's - * init parameters and cached for the lifetime of this factory. - *

- * The set is built from: - *

    - *
  • {@code jakarta.faces.FACELETS_SUFFIX} (whitespace-separated, default {@code .xhtml})
    • - *
  • Suffix entries in {@code jakarta.faces.FACELETS_VIEW_MAPPINGS} (semicolon-separated; - * entries beginning with {@code *.} contribute the extension part)
    • - *
  • {@code .jspx} — always included for legacy JSP-XML views
    • - *
- * Init parameters are read only on the first call; subsequent calls return the cached set. - */ + /** Returns cached set of allowed Facelet suffixes built from init parameters. */ private Set getAllowedSuffixes(FacesContext context) { if (_allowedSuffixes == null) { ExternalContext ec = context.getExternalContext(); - // Suffixes from jakarta.faces.FACELETS_SUFFIX (whitespace-separated, default ".xhtml") String suffixParam = ec.getInitParameter(ViewHandler.FACELETS_SUFFIX_PARAM_NAME); if (suffixParam == null) { @@ -496,7 +408,6 @@ private Set getAllowedSuffixes(FacesContext context) } Set allowed = new HashSet<>(Arrays.asList(suffixParam.trim().split("\\s+"))); - // Suffixes from jakarta.faces.FACELETS_VIEW_MAPPINGS (semicolon-separated; strip leading "*") String mappingsParam = ec.getInitParameter(ViewHandler.FACELETS_VIEW_MAPPINGS_PARAM_NAME); if (mappingsParam == null) { @@ -509,16 +420,11 @@ private Set getAllowedSuffixes(FacesContext context) token = token.trim(); if (token.startsWith("*.")) { - // suffix mapping e.g. "*.xhtml" -> ".xhtml" allowed.add(token.substring(1)); } - // Prefix mappings like "/faces/*" carry no extension — skipped. } } - // Legacy JSP-XML view support - // allowed.add(".jspx"); - if (log.isLoggable(Level.FINE)) { log.fine("Allowed Facelet suffixes: " + allowed); From 61bdf68a13cf5e6f2128bc94569de2845de99a7f Mon Sep 17 00:00:00 2001 From: Volodymyr Siedlecki Date: Thu, 13 Aug 2026 10:45:55 -0400 Subject: [PATCH 4/6] [Bug fix] Update to handle jar and wsjar resources reliably AI Assisted: Bob Version: 2.0.2 --- .../facelets/impl/DefaultFaceletFactory.java | 61 +++++++++++++------ 1 file changed, 42 insertions(+), 19 deletions(-) diff --git a/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java b/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java index f3944618d..e19ba9502 100644 --- a/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java +++ b/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java @@ -292,8 +292,21 @@ public URL resolveURL(FacesContext context, URL source, String path) throws IOEx else { // Relative path resolved against source URL - resolved = new URL(source, path); - normalizedPath = resolved.getPath(); + if (source == null) + { + // Fall back to ExternalContext if no base URL available + resolved = resolveURL(context, path); + if (resolved == null) + { + throw new FileNotFoundException("Cannot resolve relative path '" + path); + } + normalizedPath = path; + } + else + { + resolved = new URL(source, path); + normalizedPath = resolved.getPath(); + } } // Skip validation in UnitTest stage (uses synthetic paths) @@ -303,7 +316,7 @@ public URL resolveURL(FacesContext context, URL source, String path) throws IOEx } // Traversal guard: relative paths must stay within base (absolute paths already scoped by container) - if (!absoluteContextPath && !isWithinBase(resolved)) + if (!absoluteContextPath && source != null && !isWithinBase(resolved)) { throw new InvalidFileException(InvalidFileException.Reason.PATH_TRAVERSAL, "Path escapes application base: " + path); @@ -345,17 +358,6 @@ private boolean isAllowedScheme(String path) return true; } - /** Returns true if normalizedPath refers to a WEB-INF XML config file. */ - private boolean isWebInfConfigFile(String normalizedPath) - { - if (normalizedPath == null) - { - return false; - } - String lower = normalizedPath.replace('\\', '/').toLowerCase(); - return lower.contains("/web-inf/") && lower.endsWith(".xml"); - } - /** Verifies that resolved URL is contained within the application base. */ private boolean isWithinBase(URL resolved) { @@ -364,13 +366,34 @@ private boolean isWithinBase(URL resolved) { return true; } - String baseStr = base.toExternalForm(); - String resolvedStr = resolved.toExternalForm(); - if (!baseStr.endsWith("/")) + + // Compare path components (scheme-agnostic): extract path after "!" for jar URLs + String basePath = extractResourcePath(base.toExternalForm()); + String resolvedPath = extractResourcePath(resolved.toExternalForm()); + + if (!basePath.endsWith("/")) + { + basePath = basePath + "/"; + } + return resolvedPath.startsWith(basePath); + } + + /** Extract the in-archive path from jar/wsjar/file URLs (path after "!" for jar URLs). */ + private String extractResourcePath(String urlStr) + { + int jarSep = urlStr.indexOf('!'); + if (jarSep >= 0) + { + // jar: or wsjar: URL — extract path after "!" + return urlStr.substring(jarSep + 1); + } + // Regular file: URL — extract path component + int fileIdx = urlStr.indexOf("file:"); + if (fileIdx >= 0) { - baseStr = baseStr + "/"; + return urlStr.substring(fileIdx + 5); } - return resolvedStr.startsWith(baseStr); + return urlStr; } /** Returns true if normalizedPath ends with a configured Facelet extension. */ From d56b885bb4d72abc0ecec38ad355bec44cc5a78b Mon Sep 17 00:00:00 2001 From: Volodymyr Siedlecki Date: Fri, 14 Aug 2026 16:29:43 -0400 Subject: [PATCH 5/6] Use allow list over a block list --- .../facelets/impl/DefaultFaceletFactory.java | 27 +++++++------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java b/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java index e19ba9502..0786fa70a 100644 --- a/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java +++ b/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java @@ -29,7 +29,6 @@ import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; - import jakarta.el.ELException; import jakarta.enterprise.inject.spi.BeanManager; import jakarta.faces.FacesException; @@ -332,31 +331,23 @@ public URL resolveURL(FacesContext context, URL source, String path) throws IOEx return resolved; } - // Path-validation helpers - - private static final Set BLOCKED_SCHEMES = new HashSet<>( - Arrays.asList("http", "https", "ftp", "ftps", "mailto", "tel", - "imap", "irc", "nntp", "acap", "icap", "mtqp", "wss")); + // Path-validation helpers + private static final Set ALLOWED_SCHEMES = Set.of( + "file","jar","wsjar","zip"); - /** Returns false if path has a blocked scheme; true for relative/container schemes. */ + /** Returns true for relative/container schemes; false for all others */ private boolean isAllowedScheme(String path) { int colon = path.indexOf(':'); + if (colon < 1) { - return true; + return true; // relative path } + String scheme = path.substring(0, colon).toLowerCase(); - if (BLOCKED_SCHEMES.contains(scheme)) - { - if (log.isLoggable(Level.FINE)) - { - log.fine("Path not allowed [" + path + "] -> Blocked scheme: " + scheme); - } - return false; - } - return true; - } + return ALLOWED_SCHEMES.contains(scheme); + } /** Verifies that resolved URL is contained within the application base. */ private boolean isWithinBase(URL resolved) From 9df06edbe699471ad9aff7b3909541654063c39e Mon Sep 17 00:00:00 2001 From: Volodymyr Siedlecki Date: Mon, 17 Aug 2026 10:51:50 -0400 Subject: [PATCH 6/6] Create Facelet Path Validation Test Fix up AI Assisted: Bob Version: 2.0.2 --- ...faultFaceletFactoryPathValidationTest.java | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 impl/src/test/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactoryPathValidationTest.java diff --git a/impl/src/test/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactoryPathValidationTest.java b/impl/src/test/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactoryPathValidationTest.java new file mode 100644 index 000000000..477aae0cb --- /dev/null +++ b/impl/src/test/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactoryPathValidationTest.java @@ -0,0 +1,112 @@ +/* + * 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.view.facelets.impl; + +import java.net.URL; + +import jakarta.faces.application.ViewHandler; +import jakarta.faces.view.ViewDeclarationLanguage; + +import org.apache.myfaces.context.InvalidFileException; +import org.apache.myfaces.test.core.AbstractMyFacesCDIRequestTestCase; +import org.apache.myfaces.view.facelets.FaceletViewDeclarationLanguage; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +/** + * Verifies that {@link DefaultFaceletFactory#resolveURL} rejects external URLs + * and non-Facelet extensions when running outside the {@code UnitTest} ProjectStage. + * + *

The suite runs under {@code Development} stage (overriding the default + * {@code UnitTest} set by the base class) so the validation code-path is always active.

+ */ +public class DefaultFaceletFactoryPathValidationTest extends AbstractMyFacesCDIRequestTestCase +{ + @Override + protected void setUpWebConfigParams() throws Exception + { + super.setUpWebConfigParams(); + // Replace "UnitTest" so the validation branch in resolveURL is active + servletContext.addInitParameter("jakarta.faces.PROJECT_STAGE", "Development"); + } + + // ------------------------------------------------------------------------- + // Helpers + // ------------------------------------------------------------------------- + + private DefaultFaceletFactory getFactory() + { + ViewDeclarationLanguage vdl = facesContext.getApplication() + .getViewHandler() + .getViewDeclarationLanguage(facesContext, "/test.xhtml"); + return (DefaultFaceletFactory) ((FaceletViewDeclarationLanguage) vdl).getFaceletFactory(); + } + + // ------------------------------------------------------------------------- + // Tests + // ------------------------------------------------------------------------- + + /** + * An external {@code http:} URL must be rejected with + * {@link InvalidFileException.Reason#DISALLOWED_SCHEME}. + */ + @Test + public void testExternalHttpUrlIsRejected() throws Exception + { + // Only a live facesContext is needed — no view rendering required + startViewRequest(null); + + DefaultFaceletFactory factory = getFactory(); + + InvalidFileException ex = Assertions.assertThrows( + InvalidFileException.class, + () -> factory.resolveURL(facesContext, null, + "http://someverybadmaliciouswebsite.com/attack.xhtml"), + "Expected InvalidFileException for external http: URL"); + + Assertions.assertEquals(InvalidFileException.Reason.DISALLOWED_SCHEME, ex.getReason()); + + endRequest(); + } + + /** + * A relative path whose extension is not a configured Facelet suffix must + * be rejected with {@link InvalidFileException.Reason#INVALID_EXTENSION}. + */ + @Test + public void testNonFaceletExtensionIsRejected() throws Exception + { + servletContext.addInitParameter(ViewHandler.FACELETS_SUFFIX_PARAM_NAME, ".xhtml"); + startViewRequest(null); + + DefaultFaceletFactory factory = getFactory(); + + URL webappRoot = getWebappContextURI().toURL(); + URL source = new URL(webappRoot, "views/index.xhtml"); + + InvalidFileException ex = Assertions.assertThrows( + InvalidFileException.class, + () -> factory.resolveURL(facesContext, source, "template.html"), + "Expected InvalidFileException for a .html path"); + + Assertions.assertEquals(InvalidFileException.Reason.INVALID_EXTENSION, ex.getReason()); + + endRequest(); + } +}