Skip to content

Commit

Permalink
Backport fix for CVE-2021-25329
Browse files Browse the repository at this point in the history
  • Loading branch information
cesarhernandezgt committed Mar 15, 2021
1 parent d09850f commit def3ecb
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 17 deletions.
3 changes: 2 additions & 1 deletion java/org/apache/catalina/servlets/DefaultServlet.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 3 additions & 1 deletion java/org/apache/catalina/session/FileStore.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
3 changes: 2 additions & 1 deletion java/org/apache/catalina/startup/ContextConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 <b>Context</b> that configures the properties
Expand Down Expand Up @@ -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("/")) {
Expand Down
17 changes: 5 additions & 12 deletions java/org/apache/catalina/startup/ExpandWar.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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(
Expand Down Expand Up @@ -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;
Expand All @@ -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(
Expand Down
5 changes: 3 additions & 2 deletions java/org/apache/catalina/startup/HostConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()));
Expand Down
50 changes: 50 additions & 0 deletions java/org/apache/catalina/util/FileUtil.java
Original file line number Diff line number Diff line change
@@ -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);
}
}
4 changes: 4 additions & 0 deletions webapps/docs/changelog.xml
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@
Ensure the ASF logo image is correctly displayed in ROOT, docs and
host-manager applications. (violetagg)
</fix>
<scode>
Test for one directory being a sub-directory of another in a consistent
way. (markt)
</scode>
</changelog>
</subsection>
</section>
Expand Down

0 comments on commit def3ecb

Please sign in to comment.