This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 7.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/7.0.x by this push: new 74b1056 Use consistent approach for sub-directory checking 74b1056 is described below commit 74b105657ffbd1d1de80455f03446c3bbf30d1f5 Author: Mark Thomas <ma...@apache.org> AuthorDate: Wed Jan 20 13:28:57 2021 +0000 Use consistent approach for sub-directory checking --- .../apache/catalina/servlets/DefaultServlet.java | 3 +- java/org/apache/catalina/session/FileStore.java | 4 +- .../org/apache/catalina/startup/ContextConfig.java | 3 +- java/org/apache/catalina/startup/ExpandWar.java | 21 +++------ java/org/apache/catalina/startup/HostConfig.java | 5 ++- java/org/apache/catalina/util/FileUtil.java | 50 ++++++++++++++++++++++ webapps/docs/changelog.xml | 4 ++ 7 files changed, 71 insertions(+), 19 deletions(-) diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java b/java/org/apache/catalina/servlets/DefaultServlet.java index a34b2ac..e9e6332 100644 --- a/java/org/apache/catalina/servlets/DefaultServlet.java +++ b/java/org/apache/catalina/servlets/DefaultServlet.java @@ -67,6 +67,7 @@ import javax.xml.transform.stream.StreamSource; import org.apache.catalina.Globals; import org.apache.catalina.connector.RequestFacade; import org.apache.catalina.connector.ResponseFacade; +import org.apache.catalina.util.FileUtil; import org.apache.catalina.util.IOTools; import org.apache.catalina.util.RequestUtil; import org.apache.catalina.util.ServerInfo; @@ -1811,7 +1812,7 @@ public class DefaultServlet extends HttpServlet { // 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 08a02c3..4468f27 100644 --- a/java/org/apache/catalina/session/FileStore.java +++ b/java/org/apache/catalina/session/FileStore.java @@ -32,6 +32,7 @@ import javax.servlet.ServletContext; import org.apache.catalina.Context; 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; @@ -402,9 +403,10 @@ public final class FileStore extends StoreBase { 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 aff9a02..27b4a26 100644 --- a/java/org/apache/catalina/startup/ContextConfig.java +++ b/java/org/apache/catalina/startup/ContextConfig.java @@ -75,6 +75,7 @@ import org.apache.catalina.deploy.SecurityConstraint; import org.apache.catalina.deploy.ServletDef; import org.apache.catalina.deploy.WebXml; import org.apache.catalina.util.ContextName; +import org.apache.catalina.util.FileUtil; import org.apache.catalina.util.Introspection; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; @@ -761,7 +762,7 @@ public class ContextConfig implements LifecycleListener { } } - 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 a672e79..723940e 100644 --- a/java/org/apache/catalina/startup/ExpandWar.java +++ b/java/org/apache/catalina/startup/ExpandWar.java @@ -34,6 +34,7 @@ import java.util.zip.ZipException; import org.apache.catalina.Globals; import org.apache.catalina.Host; +import org.apache.catalina.util.FileUtil; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; import org.apache.tomcat.util.ExceptionUtils; @@ -98,10 +99,7 @@ public class ExpandWar { 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; @@ -114,14 +112,13 @@ public class ExpandWar { 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( sm.getString("expandWar.illegalPath",war, name, expandedFile.getCanonicalPath(), - canonicalDocBasePrefix)); + docBaseUtil.getCanonicalPath())); } int last = name.lastIndexOf('/'); if (last >= 0) { @@ -209,10 +206,7 @@ public class ExpandWar { 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; @@ -223,14 +217,13 @@ public class ExpandWar { 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( sm.getString("expandWar.illegalPath",war, name, expandedFile.getCanonicalPath(), - canonicalDocBasePrefix)); + docBaseUtil.getCanonicalPath())); } } } catch (IOException e) { diff --git a/java/org/apache/catalina/startup/HostConfig.java b/java/org/apache/catalina/startup/HostConfig.java index 50c9cb7..658d0f1 100644 --- a/java/org/apache/catalina/startup/HostConfig.java +++ b/java/org/apache/catalina/startup/HostConfig.java @@ -65,6 +65,7 @@ import org.apache.catalina.core.StandardContext; import org.apache.catalina.core.StandardHost; import org.apache.catalina.security.DeployXmlPermission; import org.apache.catalina.util.ContextName; +import org.apache.catalina.util.FileUtil; import org.apache.catalina.util.IOTools; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; @@ -690,8 +691,8 @@ public class HostConfig 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(), diff --git a/java/org/apache/catalina/util/FileUtil.java b/java/org/apache/catalina/util/FileUtil.java new file mode 100644 index 0000000..d25bde7 --- /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); + } +} diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index eec09cf..a238399 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -174,6 +174,10 @@ authority are typical examples that may need to be adjusted in some cases. (markt) </fix> + <scode> + Test for one directory being a sub-directory of another in a consistent + way. (markt) + </scode> </changelog> </subsection> <subsection name="Other"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org