From def3ecb0d30bfbca41b82fb9919c6925c5a71b3e Mon Sep 17 00:00:00 2001 From: CesarHernandezGt Date: Mon, 15 Mar 2021 14:33:08 -0600 Subject: [PATCH] Backport fix for CVE-2021-25329 --- .../catalina/servlets/DefaultServlet.java | 3 +- .../apache/catalina/session/FileStore.java | 4 +- .../catalina/startup/ContextConfig.java | 3 +- .../apache/catalina/startup/ExpandWar.java | 17 ++----- .../apache/catalina/startup/HostConfig.java | 5 +- java/org/apache/catalina/util/FileUtil.java | 50 +++++++++++++++++++ webapps/docs/changelog.xml | 4 ++ 7 files changed, 69 insertions(+), 17 deletions(-) create mode 100644 java/org/apache/catalina/util/FileUtil.java diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java b/java/org/apache/catalina/servlets/DefaultServlet.java index b1541a91f..88668aaca 100644 --- a/java/org/apache/catalina/servlets/DefaultServlet.java +++ b/java/org/apache/catalina/servlets/DefaultServlet.java @@ -61,6 +61,7 @@ import org.apache.catalina.Globals; import org.apache.catalina.connector.RequestFacade; +import org.apache.catalina.util.FileUtil; import org.apache.catalina.util.RequestUtil; import org.apache.catalina.util.ServerInfo; import org.apache.catalina.util.StringManager; @@ -1709,7 +1710,7 @@ private File validateGlobalXsltFile(File base) { // First check that the resulting path is under the provided base try { - if (!candidate.getCanonicalPath().startsWith(base.getCanonicalPath())) { + if (!(new FileUtil(base)).isParentOf(candidate)) { return null; } } catch (IOException ioe) { diff --git a/java/org/apache/catalina/session/FileStore.java b/java/org/apache/catalina/session/FileStore.java index 5086a0964..0ae142113 100644 --- a/java/org/apache/catalina/session/FileStore.java +++ b/java/org/apache/catalina/session/FileStore.java @@ -32,6 +32,7 @@ import org.apache.catalina.Globals; import org.apache.catalina.Loader; import org.apache.catalina.Session; +import org.apache.catalina.util.FileUtil; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; import org.apache.tomcat.util.res.StringManager; @@ -400,9 +401,10 @@ private File file(String id) throws IOException { } String filename = id + FILE_EXT; File file = new File(storageDir, filename); + FileUtil storageDirUtil = new FileUtil(storageDir); // Check the file is within the storage directory - if (!file.getCanonicalPath().startsWith(storageDir.getCanonicalPath())) { + if (!storageDirUtil.isParentOf(file)) { log.warn(sm.getString("fileStore.invalid", file.getPath(), id)); return null; } diff --git a/java/org/apache/catalina/startup/ContextConfig.java b/java/org/apache/catalina/startup/ContextConfig.java index 5cb9b1c2f..e48bf8947 100644 --- a/java/org/apache/catalina/startup/ContextConfig.java +++ b/java/org/apache/catalina/startup/ContextConfig.java @@ -58,6 +58,7 @@ import org.xml.sax.ErrorHandler; import org.xml.sax.InputSource; import org.xml.sax.SAXParseException; +import org.apache.catalina.util.FileUtil; /** * Startup event listener for a Context that configures the properties @@ -912,7 +913,7 @@ protected void fixDocBase() } } - if (docBase.startsWith(canonicalAppBase.getPath() + File.separatorChar)) { + if ((new FileUtil(canonicalAppBase)).isParentOf(docBase)) { docBase = docBase.substring(canonicalAppBase.getPath().length()); docBase = docBase.replace(File.separatorChar, '/'); if (docBase.startsWith("/")) { diff --git a/java/org/apache/catalina/startup/ExpandWar.java b/java/org/apache/catalina/startup/ExpandWar.java index 133053f2a..735be7bb6 100644 --- a/java/org/apache/catalina/startup/ExpandWar.java +++ b/java/org/apache/catalina/startup/ExpandWar.java @@ -32,6 +32,7 @@ import java.util.jar.JarFile; import org.apache.catalina.Host; +import org.apache.catalina.util.FileUtil; import org.apache.catalina.util.StringManager; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; @@ -136,10 +137,7 @@ public static String expand(Host host, URL war, String pathname) throw new IOException(sm.getString("expandWar.createFailed", docBase)); // Expand the WAR into the new document base directory - String canonicalDocBasePrefix = docBase.getCanonicalPath(); - if (!canonicalDocBasePrefix.endsWith(File.separator)) { - canonicalDocBasePrefix += File.separator; - } + FileUtil docBaseUtil = new FileUtil(docBase); JarURLConnection juc = (JarURLConnection) war.openConnection(); juc.setUseCaches(false); JarFile jarFile = null; @@ -152,8 +150,7 @@ public static String expand(Host host, URL war, String pathname) JarEntry jarEntry = (JarEntry) jarEntries.nextElement(); String name = jarEntry.getName(); File expandedFile = new File(docBase, name); - if (!expandedFile.getCanonicalPath().startsWith( - canonicalDocBasePrefix)) { + if (!docBaseUtil.isParentOf(expandedFile)) { // Trying to expand outside the docBase // Throw an exception to stop the deployment throw new IllegalArgumentException( @@ -241,10 +238,7 @@ public static void validate(Host host, URL war, String pathname) File docBase = new File(appBase, pathname); // Calculate the document base directory - String canonicalDocBasePrefix = docBase.getCanonicalPath(); - if (!canonicalDocBasePrefix.endsWith(File.separator)) { - canonicalDocBasePrefix += File.separator; - } + FileUtil docBaseUtil = new FileUtil(docBase); JarURLConnection juc = (JarURLConnection) war.openConnection(); juc.setUseCaches(false); JarFile jarFile = null; @@ -255,8 +249,7 @@ public static void validate(Host host, URL war, String pathname) JarEntry jarEntry = jarEntries.nextElement(); String name = jarEntry.getName(); File expandedFile = new File(docBase, name); - if (!expandedFile.getCanonicalPath().startsWith( - canonicalDocBasePrefix)) { + if (!docBaseUtil.isParentOf(expandedFile)) { // Entry located outside the docBase // Throw an exception to stop the deployment throw new IllegalArgumentException( diff --git a/java/org/apache/catalina/startup/HostConfig.java b/java/org/apache/catalina/startup/HostConfig.java index 31fcefe25..0403caafe 100644 --- a/java/org/apache/catalina/startup/HostConfig.java +++ b/java/org/apache/catalina/startup/HostConfig.java @@ -48,6 +48,7 @@ import org.apache.catalina.LifecycleListener; import org.apache.catalina.core.ContainerBase; import org.apache.catalina.core.StandardHost; +import org.apache.catalina.util.FileUtil; import org.apache.catalina.util.IOTools; import org.apache.catalina.util.StringManager; import org.apache.tomcat.util.digester.Digester; @@ -656,8 +657,8 @@ protected void deployDescriptor(String contextPath, File contextXml, String file docBase = new File(appBase(), context.getDocBase()); } // If external docBase, register .xml as redeploy first - if (!docBase.getCanonicalPath().startsWith( - appBase().getAbsolutePath() + File.separator)) { + FileUtil appBaseUtil = new FileUtil(appBase()); + if (!appBaseUtil.isParentOf(docBase)) { isExternal = true; deployedApp.redeployResources.put (contextXml.getAbsolutePath(), new Long(contextXml.lastModified())); diff --git a/java/org/apache/catalina/util/FileUtil.java b/java/org/apache/catalina/util/FileUtil.java new file mode 100644 index 000000000..10a2e3049 --- /dev/null +++ b/java/org/apache/catalina/util/FileUtil.java @@ -0,0 +1,50 @@ +/* + * 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.catalina.util; + +import java.io.File; +import java.io.IOException; + +public class FileUtil { + + private final String canonicalPath; + + public FileUtil(File f) throws IOException { + String path = f.getCanonicalPath(); + if (!path.endsWith(File.pathSeparator)) { + path += File.pathSeparatorChar; + } + + canonicalPath = path; + } + + + public String getCanonicalPath() { + return canonicalPath; + } + + + public boolean isParentOf(File child) throws IOException { + return isParentOf(child.getCanonicalPath()); + } + + + public boolean isParentOf(String childPath) { + return childPath.startsWith(canonicalPath); + } +} \ No newline at end of file diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 87b16dd59..c8c6e97ec 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -120,6 +120,10 @@ Ensure the ASF logo image is correctly displayed in ROOT, docs and host-manager applications. (violetagg) + + Test for one directory being a sub-directory of another in a consistent + way. (markt) +