This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 39c644eafa Final review of manager upload protections
39c644eafa is described below
commit 39c644eafa6a340d99c39b82eab343036a518b2a
Author: Mark Thomas <[email protected]>
AuthorDate: Fri May 8 18:14:46 2026 +0100
Final review of manager upload protections
---
.../catalina/manager/LocalStrings.properties | 4 +--
.../apache/catalina/manager/ManagerServlet.java | 39 +++++++++++++++++-----
webapps/docs/changelog.xml | 4 +--
3 files changed, 34 insertions(+), 13 deletions(-)
diff --git a/java/org/apache/catalina/manager/LocalStrings.properties
b/java/org/apache/catalina/manager/LocalStrings.properties
index 2af95f3aed..8965fbae74 100644
--- a/java/org/apache/catalina/manager/LocalStrings.properties
+++ b/java/org/apache/catalina/manager/LocalStrings.properties
@@ -158,8 +158,8 @@ managerServlet.noWrapper=Container has not called
setWrapper() for this servlet
managerServlet.notDeployed=FAIL - Context [{0}] is defined in server.xml and
may not be undeployed
managerServlet.notSslConnector=SSL is not enabled for this connector
managerServlet.objectNameFail=FAIL - Unable to register object name [{0}] for
Manager Servlet
-managerServlet.pathCheckFail=FAIL - Unable to use [{0}] as that is outside the
versioned directory [{1}]
-managerServlet.pathCheckError=FAIL - Unable to use [{0}] due to [{2}] while
checking if it was outside the versioned directory [{1}]
+managerServlet.pathCheckFail=FAIL - Unable to use [{0}] as that is outside the
expected directory [{1}]
+managerServlet.pathCheckError=FAIL - Unable to use [{0}] due to [{2}] while
checking if it was outside the expected directory [{1}]
managerServlet.postCommand=FAIL - Tried to use command [{0}] via a GET request
but POST is required
managerServlet.reloaded=OK - Reloaded application at context path [{0}]
managerServlet.renameFail=FAIL - Unable to rename [{0}] to [{1}]. This may
cause problems for future deployments.
diff --git a/java/org/apache/catalina/manager/ManagerServlet.java
b/java/org/apache/catalina/manager/ManagerServlet.java
index 6cf639f4b7..0410d26002 100644
--- a/java/org/apache/catalina/manager/ManagerServlet.java
+++ b/java/org/apache/catalina/manager/ManagerServlet.java
@@ -715,6 +715,10 @@ public class ManagerServlet extends HttpServlet implements
ContainerServlet {
}
File deployedWar = new File(host.getAppBaseFile(), baseName + ".war");
+ if (!pathCheck(deployedWar, host.getAppBaseFile(), writer, smClient)) {
+ // Any error reported in pathCheck()
+ return;
+ }
// Determine full path for uploaded WAR
File uploadedWar;
@@ -732,8 +736,8 @@ public class ManagerServlet extends HttpServlet implements
ContainerServlet {
}
} else {
File uploadPath = new File(versioned, tag);
- if (!versionedPathCheck(uploadPath, writer, smClient)) {
- // Any error reported in versionedPathCheck()
+ if (!pathCheck(uploadPath, versioned, writer, smClient)) {
+ // Any error reported in pathCheck()
return;
}
if (!uploadPath.mkdirs() && !uploadPath.isDirectory()) {
@@ -821,13 +825,16 @@ public class ManagerServlet extends HttpServlet
implements ContainerServlet {
// Find the local WAR file
File localWar = new File(new File(versioned, tag), baseName + ".war");
- if (!versionedPathCheck(localWar, writer, smClient)) {
- // Any error reported in versionedPathCheck()
+ if (!pathCheck(localWar, versioned, writer, smClient)) {
+ // Any error reported in pathCheck()
return;
}
-
File deployedWar = new File(host.getAppBaseFile(), baseName + ".war");
+ if (!pathCheck(deployedWar, host.getAppBaseFile(), writer, smClient)) {
+ // Any error reported in pathCheck()
+ return;
+ }
// Copy WAR to appBase
try {
@@ -859,14 +866,14 @@ public class ManagerServlet extends HttpServlet
implements ContainerServlet {
}
- private boolean versionedPathCheck(File input, PrintWriter writer,
StringManager smClient) {
+ private static boolean pathCheck(File input, File expected, PrintWriter
writer, StringManager smClient) {
try {
- if
(!input.getCanonicalPath().startsWith(versioned.getCanonicalPath() +
File.separator)) {
-
writer.println(smClient.getString("managerServlet.pathCheckFail", input,
versioned));
+ if
(!input.getCanonicalFile().toPath().startsWith(expected.getCanonicalFile().toPath()))
{
+
writer.println(smClient.getString("managerServlet.pathCheckFail", input,
expected));
return false;
}
} catch (IOException ioe) {
- writer.println(smClient.getString("managerServlet.pathCheckError",
input, versioned, ioe.getMessage()));
+ writer.println(smClient.getString("managerServlet.pathCheckError",
input, expected, ioe.getMessage()));
return false;
}
return true;
@@ -940,7 +947,12 @@ public class ManagerServlet extends HttpServlet implements
ContainerServlet {
return;
}
File localConfigFile = new File(configBase, baseName +
".xml");
+ if (!pathCheck(localConfigFile, configBase, writer,
smClient)) {
+ // Any error reported in pathCheck()
+ return;
+ }
File configFile = new File(config);
+
// Skip delete and copy if source == destination
if
(!configFile.getCanonicalPath().equals(localConfigFile.getCanonicalPath())) {
if (localConfigFile.isFile() &&
!localConfigFile.delete()) {
@@ -961,9 +973,18 @@ public class ManagerServlet extends HttpServlet implements
ContainerServlet {
} else {
localWarFile = new File(host.getAppBaseFile(),
baseName);
}
+ if (!pathCheck(localWarFile, host.getAppBaseFile(),
writer, smClient)) {
+ // Any error reported in pathCheck()
+ return;
+ }
+
File warFile = new File(war);
if (!warFile.isAbsolute()) {
warFile = new File(host.getAppBaseFile(), war);
+ if (!pathCheck(warFile, host.getAppBaseFile(),
writer, smClient)) {
+ // Any error reported in pathCheck()
+ return;
+ }
}
// Skip delete and copy if source == destination
if
(!warFile.getCanonicalPath().equals(localWarFile.getCanonicalPath())) {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index ae82756886..9ce88d3c3f 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -331,8 +331,8 @@
<changelog>
<!-- Entries for backport and removal before 12.0.0-M1 below this line
-->
<add>
- Manager: Add a check to ensure that any web application uploaded using
- the tag mechanism is uploaded to the correct location. (markt)
+ Manager: Add checks to ensure that any uploaded files are uploaded to
+ the expected location. (markt)
</add>
</changelog>
</subsection>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
