This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/11.0.x by this push:
new 2dd54f2caf Minor fixes from code review
2dd54f2caf is described below
commit 2dd54f2caf7450d729cdc30b8b6ee39850ded885
Author: remm <[email protected]>
AuthorDate: Mon Jul 27 15:39:58 2026 +0200
Minor fixes from code review
One validation for unexpected path traversal.
Harmonize getCreation.
---
java/org/apache/catalina/Wrapper.java | 2 +-
.../webresources/AbstractArchiveResourceSet.java | 48 ++++++++++++----------
.../catalina/webresources/ExtractingRoot.java | 16 ++++++++
.../apache/catalina/webresources/FileResource.java | 2 +-
.../apache/catalina/webresources/JarContents.java | 7 +---
.../catalina/webresources/JarResourceRoot.java | 13 +++++-
.../catalina/webresources/JarWarResourceSet.java | 10 +++++
.../catalina/webresources/LocalStrings.properties | 3 ++
.../apache/catalina/webresources/StandardRoot.java | 12 +++++-
.../webresources/war/WarURLConnection.java | 4 +-
10 files changed, 84 insertions(+), 33 deletions(-)
diff --git a/java/org/apache/catalina/Wrapper.java
b/java/org/apache/catalina/Wrapper.java
index aadb824dd9..bd6f0debd1 100644
--- a/java/org/apache/catalina/Wrapper.java
+++ b/java/org/apache/catalina/Wrapper.java
@@ -132,7 +132,7 @@ public interface Wrapper extends Container {
String[] getServletMethods() throws ServletException;
- /**
+ /**
* Returns whether this Servlet is currently unavailable.
*
* @return <code>true</code> if this Servlet is currently unavailable
diff --git
a/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java
b/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java
index ea1c43e4a8..410fb8eaa3 100644
--- a/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java
+++ b/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java
@@ -116,21 +116,24 @@ public abstract class AbstractArchiveResourceSet extends
AbstractResourceSet {
if (!pathInJar.isEmpty() && pathInJar.charAt(0) == '/') {
pathInJar = pathInJar.substring(1);
}
- for (String name : getArchiveEntries(false).keySet()) {
- if (name.length() > pathInJar.length() &&
name.startsWith(pathInJar)) {
- if (name.charAt(name.length() - 1) == '/') {
- name = name.substring(pathInJar.length(),
name.length() - 1);
- } else {
- name = name.substring(pathInJar.length());
- }
- if (name.isEmpty()) {
- continue;
- }
- if (name.charAt(0) == '/') {
- name = name.substring(1);
- }
- if (!name.isEmpty() && name.lastIndexOf('/') == -1) {
- result.add(name);
+ Map<String, JarEntry> archiveEntries = getArchiveEntries(false);
+ if (archiveEntries != null) {
+ for (String name : archiveEntries.keySet()) {
+ if (name.length() > pathInJar.length() &&
name.startsWith(pathInJar)) {
+ if (name.charAt(name.length() - 1) == '/') {
+ name = name.substring(pathInJar.length(),
name.length() - 1);
+ } else {
+ name = name.substring(pathInJar.length());
+ }
+ if (name.isEmpty()) {
+ continue;
+ }
+ if (name.charAt(0) == '/') {
+ name = name.substring(1);
+ }
+ if (!name.isEmpty() && name.lastIndexOf('/') == -1) {
+ result.add(name);
+ }
}
}
}
@@ -170,13 +173,16 @@ public abstract class AbstractArchiveResourceSet extends
AbstractResourceSet {
}
}
- for (String name : getArchiveEntries(false).keySet()) {
- if (name.length() > pathInJar.length() &&
name.startsWith(pathInJar)) {
- int nextSlash = name.indexOf('/', pathInJar.length());
- if (nextSlash != -1 && nextSlash != name.length() - 1) {
- name = name.substring(0, nextSlash + 1);
+ Map<String, JarEntry> archiveEntries = getArchiveEntries(false);
+ if (archiveEntries != null) {
+ for (String name : archiveEntries.keySet()) {
+ if (name.length() > pathInJar.length() &&
name.startsWith(pathInJar)) {
+ int nextSlash = name.indexOf('/', pathInJar.length());
+ if (nextSlash != -1 && nextSlash != name.length() - 1)
{
+ name = name.substring(0, nextSlash + 1);
+ }
+ result.add(webAppMount + '/' +
name.substring(getInternalPath().length()));
}
- result.add(webAppMount + '/' +
name.substring(getInternalPath().length()));
}
}
} else {
diff --git a/java/org/apache/catalina/webresources/ExtractingRoot.java
b/java/org/apache/catalina/webresources/ExtractingRoot.java
index a98de40870..8163d3ce9f 100644
--- a/java/org/apache/catalina/webresources/ExtractingRoot.java
+++ b/java/org/apache/catalina/webresources/ExtractingRoot.java
@@ -70,6 +70,17 @@ public class ExtractingRoot extends StandardRoot {
try {
File dest = new File(expansionTarget,
possibleJar.getName());
dest = dest.getCanonicalFile();
+ try {
+ String expansionCanonical =
expansionTarget.getCanonicalPath();
+ String destCanonical = dest.getCanonicalPath();
+ if (!destCanonical.startsWith(expansionCanonical +
File.separator) &&
+ !destCanonical.equals(expansionCanonical)) {
+ throw new LifecycleException(
+
sm.getString("extractingRoot.pathTraversal", possibleJar.getName()));
+ }
+ } catch (IOException ioe) {
+ throw new
LifecycleException(sm.getString("extractingRoot.targetFailed",
expansionTarget), ioe);
+ }
try (InputStream sourceStream =
possibleJar.getInputStream();
OutputStream destStream = new
FileOutputStream(dest)) {
IOTools.flow(sourceStream, destStream);
@@ -89,6 +100,11 @@ public class ExtractingRoot extends StandardRoot {
}
+ /**
+ * Always returns false because ExtractingRoot extracts JARs to the work
+ * directory, making the deployment behave as if it were exploded. Callers
+ * that check isPackedWarFile() should treat this as an exploded
deployment.
+ */
@Override
protected boolean isPackedWarFile() {
return false;
diff --git a/java/org/apache/catalina/webresources/FileResource.java
b/java/org/apache/catalina/webresources/FileResource.java
index 1004e97f2b..839a08b437 100644
--- a/java/org/apache/catalina/webresources/FileResource.java
+++ b/java/org/apache/catalina/webresources/FileResource.java
@@ -296,7 +296,7 @@ public class FileResource extends AbstractResource {
if (log.isDebugEnabled()) {
log.debug(sm.getString("fileResource.getCreationFail",
resource.getPath()), ioe);
}
- return 0;
+ return resource.lastModified();
}
}
diff --git a/java/org/apache/catalina/webresources/JarContents.java
b/java/org/apache/catalina/webresources/JarContents.java
index 31438bd88d..79045c7120 100644
--- a/java/org/apache/catalina/webresources/JarContents.java
+++ b/java/org/apache/catalina/webresources/JarContents.java
@@ -136,11 +136,8 @@ public final class JarContents {
for (int i = startPos; i < endPos; i++) {
h = hashPrime * h + content.charAt(i);
}
-
- if (h < 0) {
- h = h * -1;
- }
- return h;
+ // Use bitwise AND to ensure non-negative result, handles
Integer.MIN_VALUE
+ return h & Integer.MAX_VALUE;
}
diff --git a/java/org/apache/catalina/webresources/JarResourceRoot.java
b/java/org/apache/catalina/webresources/JarResourceRoot.java
index d6f1803ee3..8669bd617b 100644
--- a/java/org/apache/catalina/webresources/JarResourceRoot.java
+++ b/java/org/apache/catalina/webresources/JarResourceRoot.java
@@ -17,11 +17,14 @@
package org.apache.catalina.webresources;
import java.io.File;
+import java.io.IOException;
import java.io.InputStream;
import java.net.MalformedURLException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.attribute.BasicFileAttributes;
import java.security.cert.Certificate;
import java.util.jar.Manifest;
@@ -128,7 +131,15 @@ public class JarResourceRoot extends AbstractResource {
@Override
public long getCreation() {
- return base.lastModified();
+ try {
+ BasicFileAttributes attrs = Files.readAttributes(base.toPath(),
BasicFileAttributes.class);
+ return attrs.creationTime().toMillis();
+ } catch (IOException ioe) {
+ if (log.isDebugEnabled()) {
+ log.debug(sm.getString("fileResource.getCreationFail",
base.getPath()), ioe);
+ }
+ return base.lastModified();
+ }
}
@Override
diff --git a/java/org/apache/catalina/webresources/JarWarResourceSet.java
b/java/org/apache/catalina/webresources/JarWarResourceSet.java
index a6ab614058..3b88af6d46 100644
--- a/java/org/apache/catalina/webresources/JarWarResourceSet.java
+++ b/java/org/apache/catalina/webresources/JarWarResourceSet.java
@@ -96,6 +96,12 @@ public class JarWarResourceSet extends
AbstractArchiveResourceSet {
try {
warFile = openJarFile();
JarEntry jarFileInWar = warFile.getJarEntry(archivePath);
+ if (jarFileInWar == null) {
+ // Should never happen
+ archiveEntries = null;
+ throw new IllegalStateException(
+ sm.getString("jarWarResourceSet.jarNotFound",
archivePath, getBase()));
+ }
jarFileIs = warFile.getInputStream(jarFileInWar);
try (TomcatJarInputStream jarIs = new
TomcatJarInputStream(jarFileIs)) {
@@ -251,6 +257,10 @@ public class JarWarResourceSet extends
AbstractArchiveResourceSet {
try (JarFile warFile = new JarFile(getBase())) {
JarEntry jarFileInWar = warFile.getJarEntry(archivePath);
+ if (jarFileInWar == null) {
+ throw new LifecycleException(
+ sm.getString("jarWarResourceSet.jarNotFound",
archivePath, getBase()));
+ }
InputStream jarFileIs = warFile.getInputStream(jarFileInWar);
try (JarInputStream jarIs = new JarInputStream(jarFileIs)) {
diff --git a/java/org/apache/catalina/webresources/LocalStrings.properties
b/java/org/apache/catalina/webresources/LocalStrings.properties
index d4a23d9363..3a0f8e854f 100644
--- a/java/org/apache/catalina/webresources/LocalStrings.properties
+++ b/java/org/apache/catalina/webresources/LocalStrings.properties
@@ -56,6 +56,7 @@ dirResourceSet.startFail=Failed to start the resource set
dirResourceSet.writeNpe=The input stream may not be null
extractingRoot.jarFailed=Failed to extract the JAR file [{0}]
+extractingRoot.pathTraversal=Detected path traversal attempt in JAR name [{0}]
extractingRoot.targetFailed=Failed to create the directory [{0}] for extracted
JAR files
fileResource.getCanonicalPathFail=Unable to determine the canonical path for
the resource [{0}]
@@ -72,6 +73,7 @@ jarResourceRoot.invalidWebAppPath=This resource always refers
to a directory so
jarWarResourceSet.archiveEntriesFail=Failed to read the archive entries from
the inner JAR [{0}] located in [{1}]
jarWarResourceSet.baseUrlFail=Failed to build the base URL for the WAR file
[{0}]
jarWarResourceSet.codingError=Coding error
+jarWarResourceSet.jarNotFound=The JAR file [{0}] was not found in the WAR file
[{1}]
jarWarResourceSet.manifestFail=Failed to read the manifest from the inner JAR
[{0}] located in [{1}]
jarWarResourceSet.startFail=Failed to start the resource set
@@ -83,6 +85,7 @@ standardRoot.invalidJarUrl=The JAR/WAR URL [{0}] has invalid
syntax
standardRoot.invalidPath=The resource path [{0}] is not valid
standardRoot.invalidPathNormal=The resource path [{0}] has been normalized to
[{1}] which is not valid
standardRoot.lockedFile=The web application [{0}] failed to close the file
[{1}] opened via the following stack trace
+standardRoot.missingSeparator=Missing separator for archive path in [{0}]
standardRoot.noContext=A Context has not been configured for this
WebResourceRoot
standardRoot.startInvalidMain=The main resource set specified [{0}] is not a
directory or war file, or is not readable (it does not exist or permissions to
access it are missing)
standardRoot.unsupportedProtocol=The URL protocol [{0}] is not supported by
this web resources implementation
diff --git a/java/org/apache/catalina/webresources/StandardRoot.java
b/java/org/apache/catalina/webresources/StandardRoot.java
index b9ddc14d9f..d472fcc450 100644
--- a/java/org/apache/catalina/webresources/StandardRoot.java
+++ b/java/org/apache/catalina/webresources/StandardRoot.java
@@ -807,7 +807,7 @@ public class StandardRoot extends LifecycleMBeanBase
implements WebResourceRoot
if (f.isDirectory()) {
mainResourceSet = new DirResourceSet(this, "/",
f.getAbsolutePath(), "/");
mainResourceSet.setReadOnly(readOnly);
- } else if (f.isFile() && docBase.endsWith(".war")) {
+ } else if (f.isFile() &&
docBase.toLowerCase(Locale.ENGLISH).endsWith(".war")) {
mainResourceSet = new WarResourceSet(this, "/",
f.getAbsolutePath());
} else {
throw new
IllegalArgumentException(sm.getString("standardRoot.startInvalidMain",
f.getAbsolutePath()));
@@ -885,6 +885,9 @@ public class StandardRoot extends LifecycleMBeanBase
implements WebResourceRoot
} else {
endOfFileUrl = jarUrl.indexOf(UriUtil.getWarSeparator());
}
+ if (endOfFileUrl == -1) {
+ throw new
IllegalArgumentException(sm.getString("standardRoot.missingSeparator", jarUrl));
+ }
String fileUrl = jarUrl.substring(4, endOfFileUrl);
try {
f = new File(new URI(fileUrl));
@@ -892,7 +895,12 @@ public class StandardRoot extends LifecycleMBeanBase
implements WebResourceRoot
throw new IllegalArgumentException(
sm.getString("standardRoot.invalidJarUrl",
fileUrl), e);
}
- int startOfArchivePath = endOfFileUrl + 2;
+ int startOfArchivePath;
+ if ("jar".equals(url.getProtocol())) {
+ startOfArchivePath = endOfFileUrl + 2; // "!/" is always
2 chars
+ } else {
+ startOfArchivePath = endOfFileUrl +
UriUtil.getWarSeparator().length();
+ }
if (jarUrl.length() > startOfArchivePath) {
archivePath = jarUrl.substring(startOfArchivePath);
} else {
diff --git a/java/org/apache/catalina/webresources/war/WarURLConnection.java
b/java/org/apache/catalina/webresources/war/WarURLConnection.java
index ed1096497d..3ed14fcc08 100644
--- a/java/org/apache/catalina/webresources/war/WarURLConnection.java
+++ b/java/org/apache/catalina/webresources/war/WarURLConnection.java
@@ -26,7 +26,7 @@ import org.apache.tomcat.util.buf.UriUtil;
/**
- * URL connection for WAR resources that wraps a JAR URL connection.
+ * URL connection for WAR resources that internally wraps a JAR URL connection.
*/
public class WarURLConnection extends URLConnection {
@@ -42,7 +42,7 @@ public class WarURLConnection extends URLConnection {
/**
* Constructs a new WarURLConnection.
*
- * @param url The URL to connect to
+ * @param url The URL to connect to, which must use the "war:" protocol
*
* @throws IOException If an I/O error occurs
*/
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
