You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2021/09/09 03:15:52 UTC
[james-project] branch master updated: JAMES-3646
FileMailRepository shoud reject URL outside of James root (#637)
This is an automated email from the ASF dual-hosted git repository.
btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
The following commit(s) were added to refs/heads/master by this push:
new 5b484ce JAMES-3646 FileMailRepository shoud reject URL outside of James root (#637)
5b484ce is described below
commit 5b484ced8556790d843e56f3a60c8e3d87bc5eea
Author: Benoit TELLIER <bt...@linagora.com>
AuthorDate: Thu Sep 9 10:15:45 2021 +0700
JAMES-3646 FileMailRepository shoud reject URL outside of James root (#637)
An attacker could use webAdmin REST API to create folders in arbitrary
locations if it is not the case.
---
.../server/core/filesystem/FileSystemImpl.java | 7 ++++
.../server/core/filesystem/ResourceFactory.java | 11 +++++
.../apache/james/filesystem/api/FileSystem.java | 11 +++++
.../context/JamesServerApplicationContext.java | 8 ++++
.../web/JamesServerWebApplicationContext.java | 7 ++++
.../spring/filesystem/FileSystemImpl.java | 7 ++++
.../filesystem/ResourceLoaderFileSystem.java | 13 ++++++
.../resource/DefaultJamesResourceLoader.java | 12 ++++++
.../spring/resource/JamesResourceLoader.java | 6 +++
.../mailrepository/FileMailRepositoryTest.java | 47 ++++++++++++++++++++++
.../repository/file/AbstractFileRepository.java | 4 +-
11 files changed, 131 insertions(+), 2 deletions(-)
diff --git a/server/container/core/src/main/java/org/apache/james/server/core/filesystem/FileSystemImpl.java b/server/container/core/src/main/java/org/apache/james/server/core/filesystem/FileSystemImpl.java
index 8416c0e..94b94ae 100644
--- a/server/container/core/src/main/java/org/apache/james/server/core/filesystem/FileSystemImpl.java
+++ b/server/container/core/src/main/java/org/apache/james/server/core/filesystem/FileSystemImpl.java
@@ -54,4 +54,11 @@ public class FileSystemImpl implements FileSystem {
throw new FileNotFoundException(e.getMessage());
}
}
+
+ @Override
+ public File getFileWithinBaseDir(String fileURL) throws FileNotFoundException, IOException {
+ final File file = getFile(fileURL);
+ resourceLoader.validate(file);
+ return file;
+ }
}
diff --git a/server/container/core/src/main/java/org/apache/james/server/core/filesystem/ResourceFactory.java b/server/container/core/src/main/java/org/apache/james/server/core/filesystem/ResourceFactory.java
index 0cb9a18..bb2b740 100644
--- a/server/container/core/src/main/java/org/apache/james/server/core/filesystem/ResourceFactory.java
+++ b/server/container/core/src/main/java/org/apache/james/server/core/filesystem/ResourceFactory.java
@@ -19,6 +19,7 @@
package org.apache.james.server.core.filesystem;
import java.io.File;
+import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
@@ -32,6 +33,16 @@ public class ResourceFactory {
public ResourceFactory(JamesDirectoriesProvider directoryProvider) {
this.directoryProvider = directoryProvider;
}
+
+ public void validate(File file) throws IOException {
+ String canonicalPath = file.getCanonicalPath();
+ if (!canonicalPath.startsWith(directoryProvider.getAbsoluteDirectory())
+ && !canonicalPath.startsWith(directoryProvider.getRootDirectory())
+ && !canonicalPath.startsWith(directoryProvider.getVarDirectory())) {
+
+ throw new IOException(canonicalPath + " jail break outside of " + directoryProvider.getRootDirectory());
+ }
+ }
public Resource getResource(String fileURL) {
if (fileURL.startsWith(FileSystem.CLASSPATH_PROTOCOL)) {
diff --git a/server/container/filesystem-api/src/main/java/org/apache/james/filesystem/api/FileSystem.java b/server/container/filesystem-api/src/main/java/org/apache/james/filesystem/api/FileSystem.java
index 2221dfa..96f095b 100644
--- a/server/container/filesystem-api/src/main/java/org/apache/james/filesystem/api/FileSystem.java
+++ b/server/container/filesystem-api/src/main/java/org/apache/james/filesystem/api/FileSystem.java
@@ -82,6 +82,17 @@ public interface FileSystem {
File getFile(String fileURL) throws FileNotFoundException;
/**
+ * Similar to getFile but enforces the file to be within baseDir
+ */
+ default File getFileWithinBaseDir(String fileURL) throws FileNotFoundException, IOException {
+ File file = getFile(fileURL);
+ if (file.getCanonicalPath().startsWith(getBasedir().getCanonicalPath())) {
+ return file;
+ }
+ throw new IOException(fileURL + " -> " + file.getCanonicalPath() + " jail break outside of " + getBasedir().getCanonicalPath());
+ }
+
+ /**
* Return the base folder used by the application
*/
File getBasedir() throws FileNotFoundException;
diff --git a/server/container/spring/src/main/java/org/apache/james/container/spring/context/JamesServerApplicationContext.java b/server/container/spring/src/main/java/org/apache/james/container/spring/context/JamesServerApplicationContext.java
index b97f124..ec7f0e9 100644
--- a/server/container/spring/src/main/java/org/apache/james/container/spring/context/JamesServerApplicationContext.java
+++ b/server/container/spring/src/main/java/org/apache/james/container/spring/context/JamesServerApplicationContext.java
@@ -18,6 +18,9 @@
****************************************************************/
package org.apache.james.container.spring.context;
+import java.io.File;
+import java.io.IOException;
+
import org.apache.james.container.spring.resource.DefaultJamesResourceLoader;
import org.apache.james.container.spring.resource.JamesResourceLoader;
import org.apache.james.server.core.JamesServerResourceLoader;
@@ -40,6 +43,11 @@ public class JamesServerApplicationContext extends ClassPathXmlApplicationContex
super(configs);
}
+ @Override
+ public void validate(File file) throws IOException {
+ resourceLoader.validate(file);
+ }
+
/**
* Protected accessor for the resource loader.
*/
diff --git a/server/container/spring/src/main/java/org/apache/james/container/spring/context/web/JamesServerWebApplicationContext.java b/server/container/spring/src/main/java/org/apache/james/container/spring/context/web/JamesServerWebApplicationContext.java
index 4a6f22c..f458288 100644
--- a/server/container/spring/src/main/java/org/apache/james/container/spring/context/web/JamesServerWebApplicationContext.java
+++ b/server/container/spring/src/main/java/org/apache/james/container/spring/context/web/JamesServerWebApplicationContext.java
@@ -18,6 +18,8 @@
****************************************************************/
package org.apache.james.container.spring.context.web;
+import java.io.File;
+import java.io.IOException;
import java.util.Objects;
import org.apache.james.container.spring.resource.DefaultJamesResourceLoader;
@@ -79,6 +81,11 @@ public class JamesServerWebApplicationContext extends XmlWebApplicationContext i
private String confDirectory;
@Override
+ public void validate(File file) throws IOException {
+ resourceLoader.validate(file);
+ }
+
+ @Override
public Resource getResource(String fileURL) {
// delegate the loading to the resourceloader
Resource r = resourceLoader.getResource(fileURL);
diff --git a/server/container/spring/src/main/java/org/apache/james/container/spring/filesystem/FileSystemImpl.java b/server/container/spring/src/main/java/org/apache/james/container/spring/filesystem/FileSystemImpl.java
index 5b788c4..cf4126d 100644
--- a/server/container/spring/src/main/java/org/apache/james/container/spring/filesystem/FileSystemImpl.java
+++ b/server/container/spring/src/main/java/org/apache/james/container/spring/filesystem/FileSystemImpl.java
@@ -61,4 +61,11 @@ public class FileSystemImpl implements FileSystem, ApplicationContextAware {
this.resourceLoader = (JamesResourceLoader) context;
}
+ @Override
+ public File getFileWithinBaseDir(String fileURL) throws IOException {
+ File file = getFile(fileURL);
+ resourceLoader.validate(file);
+ return file;
+ }
+
}
diff --git a/server/container/spring/src/main/java/org/apache/james/container/spring/filesystem/ResourceLoaderFileSystem.java b/server/container/spring/src/main/java/org/apache/james/container/spring/filesystem/ResourceLoaderFileSystem.java
index 996010b..cb3ca7b 100644
--- a/server/container/spring/src/main/java/org/apache/james/container/spring/filesystem/ResourceLoaderFileSystem.java
+++ b/server/container/spring/src/main/java/org/apache/james/container/spring/filesystem/ResourceLoaderFileSystem.java
@@ -23,6 +23,7 @@ import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
+import org.apache.james.container.spring.resource.JamesResourceLoader;
import org.apache.james.filesystem.api.FileSystem;
import org.springframework.context.ResourceLoaderAware;
import org.springframework.core.io.ResourceLoader;
@@ -54,6 +55,18 @@ public class ResourceLoaderFileSystem implements FileSystem, ResourceLoaderAware
}
}
+
+ @Override
+ public File getFileWithinBaseDir(String fileURL) throws IOException {
+ File file = getFile(fileURL);
+ if (loader instanceof JamesResourceLoader) {
+ ((JamesResourceLoader) loader).validate(file);
+ return file;
+ } else {
+ return FileSystem.super.getFileWithinBaseDir(fileURL);
+ }
+ }
+
@Override
public void setResourceLoader(ResourceLoader loader) {
this.loader = loader;
diff --git a/server/container/spring/src/main/java/org/apache/james/container/spring/resource/DefaultJamesResourceLoader.java b/server/container/spring/src/main/java/org/apache/james/container/spring/resource/DefaultJamesResourceLoader.java
index cec07a6..1773a85 100644
--- a/server/container/spring/src/main/java/org/apache/james/container/spring/resource/DefaultJamesResourceLoader.java
+++ b/server/container/spring/src/main/java/org/apache/james/container/spring/resource/DefaultJamesResourceLoader.java
@@ -19,6 +19,7 @@
package org.apache.james.container.spring.resource;
import java.io.File;
+import java.io.IOException;
import org.apache.james.filesystem.api.FileSystem;
import org.apache.james.filesystem.api.JamesDirectoriesProvider;
@@ -39,6 +40,17 @@ public class DefaultJamesResourceLoader extends DefaultResourceLoader implements
public DefaultJamesResourceLoader(JamesDirectoriesProvider jamesDirectoriesProvider) {
this.jamesDirectoriesProvider = jamesDirectoriesProvider;
}
+
+ @Override
+ public void validate(File file) throws IOException {
+ String canonicalPath = file.getCanonicalPath();
+ if (!canonicalPath.startsWith(jamesDirectoriesProvider.getAbsoluteDirectory())
+ && !canonicalPath.startsWith(jamesDirectoriesProvider.getRootDirectory())
+ && !canonicalPath.startsWith(jamesDirectoriesProvider.getVarDirectory())) {
+
+ throw new IOException(canonicalPath + " jail break outside of " + jamesDirectoriesProvider.getRootDirectory());
+ }
+ }
/**
* Return the {@link Resource} for the given url. If the resource can not be
diff --git a/server/container/spring/src/main/java/org/apache/james/container/spring/resource/JamesResourceLoader.java b/server/container/spring/src/main/java/org/apache/james/container/spring/resource/JamesResourceLoader.java
index 7543c63..dd0f37e 100644
--- a/server/container/spring/src/main/java/org/apache/james/container/spring/resource/JamesResourceLoader.java
+++ b/server/container/spring/src/main/java/org/apache/james/container/spring/resource/JamesResourceLoader.java
@@ -18,6 +18,9 @@
****************************************************************/
package org.apache.james.container.spring.resource;
+import java.io.File;
+import java.io.IOException;
+
import org.apache.james.filesystem.api.JamesDirectoriesProvider;
import org.springframework.core.io.ResourceLoader;
@@ -26,5 +29,8 @@ import org.springframework.core.io.ResourceLoader;
* important Directories, which are in use by JAMES.
*/
public interface JamesResourceLoader extends ResourceLoader, JamesDirectoriesProvider {
+ default void validate(File file) throws IOException {
+
+ }
}
diff --git a/server/data/data-file/src/test/java/org/apache/james/mailrepository/FileMailRepositoryTest.java b/server/data/data-file/src/test/java/org/apache/james/mailrepository/FileMailRepositoryTest.java
index ec5a39c..c9e454e 100644
--- a/server/data/data-file/src/test/java/org/apache/james/mailrepository/FileMailRepositoryTest.java
+++ b/server/data/data-file/src/test/java/org/apache/james/mailrepository/FileMailRepositoryTest.java
@@ -19,6 +19,10 @@
package org.apache.james.mailrepository;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.io.IOException;
+
import org.apache.commons.configuration2.BaseHierarchicalConfiguration;
import org.apache.james.filesystem.api.mock.MockFileSystem;
import org.apache.james.mailrepository.api.MailRepository;
@@ -27,6 +31,7 @@ import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
public class FileMailRepositoryTest {
@@ -75,6 +80,48 @@ public class FileMailRepositoryTest {
}
@Nested
+ @DisplayName("Default configuration")
+ public class AccessTest {
+ private FileMailRepository mailRepository;
+ private MockFileSystem filesystem;
+
+ @BeforeEach
+ void init() throws Exception {
+ filesystem = new MockFileSystem();
+ mailRepository = new FileMailRepository();
+ mailRepository.setFileSystem(filesystem);
+ }
+
+ protected BaseHierarchicalConfiguration getConfiguration() {
+ BaseHierarchicalConfiguration configuration = new BaseHierarchicalConfiguration();
+ configuration.addProperty("[@destinationURL]", "file://../../trying/a/location/of/james/root");
+ return withConfigurationOptions(configuration);
+ }
+
+ protected BaseHierarchicalConfiguration withConfigurationOptions(BaseHierarchicalConfiguration configuration) {
+ configuration.addProperty("[@FIFO]", "false");
+ configuration.addProperty("[@CACHEKEYS]", "true");
+ return configuration;
+ }
+
+ @AfterEach
+ void tearDown() {
+ filesystem.clear();
+ }
+
+ @Test
+ void repositoriesOutsideOfJamesRootShouldBeRejected() {
+ assertThatThrownBy(() -> {
+ mailRepository.configure(getConfiguration());
+ mailRepository.init();
+ }).isInstanceOf(org.apache.commons.configuration2.ex.ConfigurationException.class)
+ .hasCauseInstanceOf(IOException.class)
+ .<Throwable>extracting(Throwable::getCause)
+ .satisfies(e -> e.getMessage().startsWith("file://../../trying/a/location/of/james/root jail break outside of "));
+ }
+ }
+
+ @Nested
@DisplayName("No cache configuration")
public class NoCacheFileMailRepositoryTest extends GenericFileMailRepositoryTest {
diff --git a/server/data/data-library/src/main/java/org/apache/james/repository/file/AbstractFileRepository.java b/server/data/data-library/src/main/java/org/apache/james/repository/file/AbstractFileRepository.java
index d3e4ab1..1e6b70e 100644
--- a/server/data/data-library/src/main/java/org/apache/james/repository/file/AbstractFileRepository.java
+++ b/server/data/data-library/src/main/java/org/apache/james/repository/file/AbstractFileRepository.java
@@ -146,8 +146,8 @@ public abstract class AbstractFileRepository implements Configurable {
}
try {
- baseDirectory = fileSystem.getFile(destination);
- } catch (FileNotFoundException e) {
+ baseDirectory = fileSystem.getFileWithinBaseDir(destination);
+ } catch (IOException e) {
throw new ConfigurationException("Unable to acces destination " + destination, e);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org