This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push: new bb33048 Improve validation of storage location when using FileStore. bb33048 is described below commit bb33048e3f9b4f2b70e4da2e6c4e34ca89023b1b Author: Mark Thomas <ma...@apache.org> AuthorDate: Tue May 5 15:50:15 2020 +0100 Improve validation of storage location when using FileStore. --- java/org/apache/catalina/session/FileStore.java | 19 +++++++++++++++++-- .../apache/catalina/session/LocalStrings.properties | 1 + webapps/docs/changelog.xml | 3 +++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/java/org/apache/catalina/session/FileStore.java b/java/org/apache/catalina/session/FileStore.java index 0d0f41c..5c88e49 100644 --- a/java/org/apache/catalina/session/FileStore.java +++ b/java/org/apache/catalina/session/FileStore.java @@ -33,6 +33,8 @@ import org.apache.catalina.Context; import org.apache.catalina.Globals; import org.apache.catalina.Session; import org.apache.juli.logging.Log; +import org.apache.juli.logging.LogFactory; +import org.apache.tomcat.util.res.StringManager; /** * Concrete implementation of the <b>Store</b> interface that utilizes @@ -43,6 +45,10 @@ import org.apache.juli.logging.Log; */ public final class FileStore extends StoreBase { + private static final Log log = LogFactory.getLog(FileStore.class); + private static final StringManager sm = StringManager.getManager(FileStore.class); + + // ----------------------------------------------------- Constants /** @@ -336,11 +342,20 @@ public final class FileStore extends StoreBase { * used in the file naming. */ private File file(String id) throws IOException { - if (this.directory == null) { + File storageDir = directory(); + if (storageDir == null) { return null; } + String filename = id + FILE_EXT; - File file = new File(directory(), filename); + File file = new File(storageDir, filename); + + // Check the file is within the storage directory + if (!file.getCanonicalPath().startsWith(storageDir.getCanonicalPath())) { + log.warn(sm.getString("fileStore.invalid", file.getPath(), id)); + return null; + } + return file; } } diff --git a/java/org/apache/catalina/session/LocalStrings.properties b/java/org/apache/catalina/session/LocalStrings.properties index 62a8b49..127cd43 100644 --- a/java/org/apache/catalina/session/LocalStrings.properties +++ b/java/org/apache/catalina/session/LocalStrings.properties @@ -29,6 +29,7 @@ JDBCStore.wrongDataSource=Cannot open JNDI DataSource [{0}] fileStore.createFailed=Unable to create directory [{0}] for the storage of session data fileStore.deleteFailed=Unable to delete file [{0}] which is preventing the creation of the session storage location fileStore.deleteSessionFailed=Unable to delete file [{0}] which is no longer required +fileStore.invalid=Invalid persistence file [{0}] for session ID [{1}] fileStore.loading=Loading Session [{0}] from file [{1}] fileStore.removing=Removing Session [{0}] at file [{1}] fileStore.saving=Saving Session [{0}] to file [{1}] diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index ac0f7e0..c47f1f4 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -114,6 +114,9 @@ replacement to <code>:-</code> due to possible conflicts. The syntax is now <code>${name:-default}</code>. (remm) </fix> + <add> + Improve validation of storage location when using FileStore. (markt) + </add> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org