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

markt-asf pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/9.0.x by this push:
     new 19c141475a Additional context path validation on deployment
19c141475a is described below

commit 19c141475a27190c8f6b0ce9552b1627df05932f
Author: Mark Thomas <[email protected]>
AuthorDate: Tue May 12 12:24:43 2026 +0100

    Additional context path validation on deployment
---
 .../catalina/manager/HTMLManagerServlet.java       |  6 ++
 .../apache/catalina/manager/ManagerServlet.java    |  4 +-
 java/org/apache/catalina/startup/HostConfig.java   | 77 ++++++++++------------
 .../catalina/startup/LocalStrings.properties       |  2 +
 java/org/apache/catalina/util/ContextName.java     | 14 ++++
 webapps/docs/changelog.xml                         |  4 ++
 6 files changed, 60 insertions(+), 47 deletions(-)

diff --git a/java/org/apache/catalina/manager/HTMLManagerServlet.java 
b/java/org/apache/catalina/manager/HTMLManagerServlet.java
index 54c59a769f..2457da654d 100644
--- a/java/org/apache/catalina/manager/HTMLManagerServlet.java
+++ b/java/org/apache/catalina/manager/HTMLManagerServlet.java
@@ -248,6 +248,12 @@ public class HTMLManagerServlet extends ManagerServlet {
                 }
 
                 ContextName cn = new ContextName(filename, true);
+                StringWriter stringWriter = new StringWriter();
+                PrintWriter printWriter = new PrintWriter(stringWriter);
+                if (!validateContextName(cn, printWriter, smClient)) {
+                    return stringWriter.toString();
+                }
+
                 String name = cn.getName();
 
                 if (host.findChild(name) != null && !isDeployed(name)) {
diff --git a/java/org/apache/catalina/manager/ManagerServlet.java 
b/java/org/apache/catalina/manager/ManagerServlet.java
index 28e7de8f2d..e45dfaccc8 100644
--- a/java/org/apache/catalina/manager/ManagerServlet.java
+++ b/java/org/apache/catalina/manager/ManagerServlet.java
@@ -1639,9 +1639,7 @@ public class ManagerServlet extends HttpServlet 
implements ContainerServlet {
      */
     protected static boolean validateContextName(ContextName cn, PrintWriter 
writer, StringManager smClient) {
 
-        // ContextName should be non-null with a path that is empty or starts
-        // with /
-        if (cn != null && (cn.getPath().startsWith("/") || 
cn.getPath().isEmpty())) {
+        if (cn != null && cn.isPathValid()) {
             return true;
         }
 
diff --git a/java/org/apache/catalina/startup/HostConfig.java 
b/java/org/apache/catalina/startup/HostConfig.java
index df21dca8e3..eb39c52993 100644
--- a/java/org/apache/catalina/startup/HostConfig.java
+++ b/java/org/apache/catalina/startup/HostConfig.java
@@ -164,6 +164,16 @@ public class HostConfig implements LifecycleListener {
     protected Digester digester = createDigester(contextClass);
     private final Object digesterLock = new Object();
 
+    /**
+     * The list of descriptors in the appBase to be ignored because they are 
invalid (e.g. contain /../ sequences).
+     */
+    protected final Set<String> invalidDescriptors = new HashSet<>();
+
+    /**
+     * The list of directories in the appBase to be ignored because they are 
invalid (e.g. contain /../ sequences).
+     */
+    protected final Set<String> invalidDirectories = new HashSet<>();
+
     /**
      * The list of Wars in the appBase to be ignored because they are invalid 
(e.g. contain /../ sequences).
      */
@@ -580,7 +590,7 @@ public class HostConfig implements LifecycleListener {
         for (String file : files) {
             File contextXml = new File(configBase, file);
 
-            if (file.toLowerCase(Locale.ENGLISH).endsWith(".xml")) {
+            if (file.toLowerCase(Locale.ENGLISH).endsWith(".xml") && 
!invalidDescriptors.contains(file)) {
                 ContextName cn = new ContextName(file, true);
 
                 if (tryAddServiced(cn.getName())) {
@@ -622,6 +632,13 @@ public class HostConfig implements LifecycleListener {
     @SuppressWarnings("null") // context is not null
     protected void deployDescriptor(ContextName cn, File contextXml) {
 
+        // Check for descriptors with /../ /./ or similar sequences in the name
+        if (!cn.isPathValid()) {
+            log.error(sm.getString("hostConfig.illegalDescriptorName", 
contextXml.getName()));
+            invalidDescriptors.add(contextXml.getName());
+            return;
+        }
+
         DeployedApplication deployedApp = new 
DeployedApplication(cn.getName(), true);
 
         long startTime = 0;
@@ -824,14 +841,6 @@ public class HostConfig implements LifecycleListener {
                             continue;
                         }
 
-                        // Check for WARs with /../ /./ or similar sequences 
in the name
-                        if (!validateContextPath(appBase, cn.getBaseName())) {
-                            
log.error(sm.getString("hostConfig.illegalWarName", file));
-                            invalidWars.add(file);
-                            removeServiced(cn.getName());
-                            continue;
-                        }
-
                         // DeployWAR will call removeServiced
                         results.add(es.submit(new DeployWar(this, cn, war)));
                     } catch (Throwable t) {
@@ -853,40 +862,6 @@ public class HostConfig implements LifecycleListener {
     }
 
 
-    private boolean validateContextPath(File appBase, String contextPath) {
-        // More complicated than the ideal as the canonical path may or may
-        // not end with File.separator for a directory
-
-        StringBuilder docBase;
-        String canonicalDocBase;
-
-        try {
-            String canonicalAppBase = appBase.getCanonicalPath();
-            docBase = new StringBuilder(canonicalAppBase);
-            if (canonicalAppBase.endsWith(File.separator)) {
-                docBase.append(contextPath.substring(1).replace('/', 
File.separatorChar));
-            } else {
-                docBase.append(contextPath.replace('/', File.separatorChar));
-            }
-            // At this point docBase should be canonical but will not end
-            // with File.separator
-
-            canonicalDocBase = (new 
File(docBase.toString())).getCanonicalPath();
-
-            // If the canonicalDocBase ends with File.separator, add one to
-            // docBase before they are compared
-            if (canonicalDocBase.endsWith(File.separator)) {
-                docBase.append(File.separator);
-            }
-        } catch (IOException ioe) {
-            return false;
-        }
-
-        // Compare the two. If they are not the same, the contextPath must
-        // have /../ like sequences in it
-        return canonicalDocBase.contentEquals(docBase);
-    }
-
     /**
      * Deploy packed WAR.
      * <p>
@@ -897,6 +872,13 @@ public class HostConfig implements LifecycleListener {
      */
     protected void deployWAR(ContextName cn, File war) {
 
+        // Check for WARs with /../ /./ or similar sequences in the name
+        if (!cn.isPathValid()) {
+            log.error(sm.getString("hostConfig.illegalWarName", 
war.getName()));
+            invalidWars.add(war.getName());
+            return;
+        }
+
         File xml = new File(host.getAppBaseFile(), cn.getBaseName() + "/" + 
Constants.ApplicationContextXml);
 
         File warTracker = new File(host.getAppBaseFile(), cn.getBaseName() + 
Constants.WarTracker);
@@ -1090,7 +1072,7 @@ public class HostConfig implements LifecycleListener {
             }
 
             File dir = new File(appBase, file);
-            if (dir.isDirectory()) {
+            if (dir.isDirectory() && !invalidDirectories.contains(file)) {
                 ContextName cn = new ContextName(file, false);
 
                 if (tryAddServiced(cn.getName())) {
@@ -1131,6 +1113,13 @@ public class HostConfig implements LifecycleListener {
      */
     protected void deployDirectory(ContextName cn, File dir) {
 
+        // Check for directories with /../ /./ or similar sequences in the name
+        if (!cn.isPathValid()) {
+            log.error(sm.getString("hostConfig.illegalDirName", 
dir.getName()));
+            invalidDirectories.add(dir.getName());
+            return;
+        }
+
         long startTime = 0;
         // Deploy the application in this directory
         if (log.isInfoEnabled()) {
diff --git a/java/org/apache/catalina/startup/LocalStrings.properties 
b/java/org/apache/catalina/startup/LocalStrings.properties
index a11fd3e763..ca20456c19 100644
--- a/java/org/apache/catalina/startup/LocalStrings.properties
+++ b/java/org/apache/catalina/startup/LocalStrings.properties
@@ -145,6 +145,8 @@ hostConfig.docBaseUrlInvalid=The provided docBase cannot be 
expressed as a URL
 hostConfig.expand=Expanding web application archive [{0}]
 hostConfig.expand.error=Exception while expanding web application archive [{0}]
 hostConfig.ignorePath=Ignoring path [{0}] in appBase for automatic deployment
+hostConfig.illegalDescriptorName=The descriptor name [{0}] is invalid. The 
archive will be ignored.
+hostConfig.illegalDirName=The directory name [{0}] is invalid. The archive 
will be ignored.
 hostConfig.illegalWarName=The war name [{0}] is invalid. The archive will be 
ignored.
 hostConfig.jmx.register=Register context [{0}] failed
 hostConfig.jmx.unregister=Unregister context [{0}] failed
diff --git a/java/org/apache/catalina/util/ContextName.java 
b/java/org/apache/catalina/util/ContextName.java
index 911505da9a..54014c37f4 100644
--- a/java/org/apache/catalina/util/ContextName.java
+++ b/java/org/apache/catalina/util/ContextName.java
@@ -18,6 +18,8 @@ package org.apache.catalina.util;
 
 import java.util.Locale;
 
+import org.apache.tomcat.util.http.RequestUtil;
+
 /**
  * Utility class to manage context names so there is one place where the 
conversions between baseName, path and version
  * take place.
@@ -199,6 +201,18 @@ public final class ContextName {
     }
 
 
+    public boolean isPathValid() {
+        // No need to test for null since path can never be null (see 
constructors)
+        //
+        // Therefore, just need to check:
+        // - empty or start with /
+        // - normalized and no attempt to escape the root
+        //
+        // Note: Normalize check on empty path would fail
+        return path.isEmpty() || (path.startsWith("/") && 
path.equals(RequestUtil.normalize(path)));
+    }
+
+
     /**
      * Extract the final component of the given path which is assumed to be a 
base name and generate a
      * {@link ContextName} from that base name.
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 3bf434898e..69cd411b69 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -126,6 +126,10 @@
         Manager: Add checks to ensure that any uploaded files are uploaded to
         the expected location. (markt)
       </add>
+      <add>
+        Manager: Add checks to ensure that the requested context path for a
+        deployed WAR, directory or descriptor file is valid. (markt)
+      </add>
     </changelog>
   </subsection>
   <subsection name="Other">


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to