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:21 UTC
[james-project] branch master updated (1133568 -> 115c9f8)
This is an automated email from the ASF dual-hosted git repository.
btellier pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git.
from 1133568 JAMES-3150 BloomFilterGCAlgorithmContract: Lower false positive rates (#644)
new b25c243 JAMES-3646 SieveFileRepository should validate underlying files belong to its root
new 115c9f8 JAMES-3646 SieveFileRepository should forbid '/' usage
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../sieverepository/file/SieveFileRepository.java | 29 +++++++++++++++++++---
.../file/SieveFileRepositoryTest.java | 29 ++++++++++++++++++++++
2 files changed, 55 insertions(+), 3 deletions(-)
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[james-project] 02/02: JAMES-3646 SieveFileRepository should forbid
'/' usage
Posted by bt...@apache.org.
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
commit 115c9f8db650562407de004e343f1778a70a48de
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Sun Sep 5 08:05:37 2021 +0700
JAMES-3646 SieveFileRepository should forbid '/' usage
This could allow one user to read scripts of the other...
---
.../apache/james/sieverepository/file/SieveFileRepository.java | 3 +++
.../james/sieverepository/file/SieveFileRepositoryTest.java | 10 ++++++++++
2 files changed, 13 insertions(+)
diff --git a/server/data/data-file/src/main/java/org/apache/james/sieverepository/file/SieveFileRepository.java b/server/data/data-file/src/main/java/org/apache/james/sieverepository/file/SieveFileRepository.java
index 34425ed..26baf98 100644
--- a/server/data/data-file/src/main/java/org/apache/james/sieverepository/file/SieveFileRepository.java
+++ b/server/data/data-file/src/main/java/org/apache/james/sieverepository/file/SieveFileRepository.java
@@ -377,6 +377,9 @@ public class SieveFileRepository implements SieveRepository {
}
protected File getScriptFile(Username username, ScriptName name) throws ScriptNotFoundException, StorageException {
+ if (name.getValue().contains("/")) {
+ throw new StorageException(new IllegalArgumentException("Script name should not contain '/' as it can allow path traversal"));
+ }
File file = new File(getUserDirectory(username), name.getValue());
enforceRoot(file);
if (!file.exists()) {
diff --git a/server/data/data-file/src/test/java/org/apache/james/sieverepository/file/SieveFileRepositoryTest.java b/server/data/data-file/src/test/java/org/apache/james/sieverepository/file/SieveFileRepositoryTest.java
index f9afe9f..e6b4f17 100644
--- a/server/data/data-file/src/test/java/org/apache/james/sieverepository/file/SieveFileRepositoryTest.java
+++ b/server/data/data-file/src/test/java/org/apache/james/sieverepository/file/SieveFileRepositoryTest.java
@@ -12,6 +12,7 @@ import java.io.InputStream;
import org.apache.commons.io.FileUtils;
import org.apache.james.core.Username;
import org.apache.james.filesystem.api.FileSystem;
+import org.apache.james.sieverepository.api.ScriptContent;
import org.apache.james.sieverepository.api.ScriptName;
import org.apache.james.sieverepository.api.SieveRepository;
import org.apache.james.sieverepository.api.exception.StorageException;
@@ -74,4 +75,13 @@ class SieveFileRepositoryTest implements SieveRepositoryContract {
new ScriptName("../../../../home/interview1/script"), SCRIPT_CONTENT))
.isInstanceOf(StorageException.class);
}
+
+ @Test
+ void getScriptShouldNotAllowToReadScriptsOfOtherUsers() throws Exception {
+ sieveRepository().putScript(Username.of("other"), new ScriptName("script"), new ScriptContent("PWND!!!"));
+
+ assertThatThrownBy(() -> sieveRepository().getScript(Username.of("test"),
+ new ScriptName("../other/script")))
+ .isInstanceOf(StorageException.class);
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[james-project] 01/02: JAMES-3646 SieveFileRepository should
validate underlying files belong to its root
Posted by bt...@apache.org.
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
commit b25c243a265129b999fdead379230ee7ae4c6cc7
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Sun Sep 5 07:55:33 2021 +0700
JAMES-3646 SieveFileRepository should validate underlying files belong to its root
---
.../sieverepository/file/SieveFileRepository.java | 26 +++++++++++++++++++---
.../file/SieveFileRepositoryTest.java | 19 ++++++++++++++++
2 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/server/data/data-file/src/main/java/org/apache/james/sieverepository/file/SieveFileRepository.java b/server/data/data-file/src/main/java/org/apache/james/sieverepository/file/SieveFileRepository.java
index 832e6c8..34425ed 100644
--- a/server/data/data-file/src/main/java/org/apache/james/sieverepository/file/SieveFileRepository.java
+++ b/server/data/data-file/src/main/java/org/apache/james/sieverepository/file/SieveFileRepository.java
@@ -76,6 +76,7 @@ public class SieveFileRepository implements SieveRepository {
public static final String SIEVE_EXTENSION = ".sieve";
private final FileSystem fileSystem;
+ private final File root;
private final Object lock = new Object();
/**
@@ -140,7 +141,7 @@ public class SieveFileRepository implements SieveRepository {
@Inject
public SieveFileRepository(FileSystem fileSystem) throws IOException {
this.fileSystem = fileSystem;
- File root = fileSystem.getFile(SIEVE_ROOT);
+ this.root = fileSystem.getFile(SIEVE_ROOT);
FileUtils.forceMkdir(root);
}
@@ -228,6 +229,7 @@ public class SieveFileRepository implements SieveRepository {
public void putScript(Username username, ScriptName name, ScriptContent content) throws StorageException, QuotaExceededException {
synchronized (lock) {
File file = new File(getUserDirectory(username), name.getValue());
+ enforceRoot(file);
haveSpace(username, name, content.length());
toFile(file, content.getValue());
}
@@ -239,6 +241,7 @@ public class SieveFileRepository implements SieveRepository {
synchronized (lock) {
File oldFile = getScriptFile(username, oldName);
File newFile = new File(getUserDirectory(username), newName.getValue());
+ enforceRoot(newFile);
if (newFile.exists()) {
throw new DuplicateException("User: " + username.asString() + "Script: " + newName);
}
@@ -312,8 +315,20 @@ public class SieveFileRepository implements SieveRepository {
return file;
}
+ private void enforceRoot(File file) throws StorageException {
+ try {
+ if (!file.getCanonicalPath().startsWith(root.getCanonicalPath())) {
+ throw new StorageException(new IllegalStateException("Path traversal attempted"));
+ }
+ } catch (IOException e) {
+ throw new StorageException(e);
+ }
+ }
+
protected File getUserDirectoryFile(Username username) throws StorageException {
- return new File(getSieveRootDirectory(), username.asString() + '/');
+ final File userFile = new File(getSieveRootDirectory(), username.asString() + '/');
+ enforceRoot(userFile);
+ return userFile;
}
protected File getActiveFile(Username username) throws ScriptNotFoundException, StorageException {
@@ -324,7 +339,9 @@ public class SieveFileRepository implements SieveRepository {
} catch (FileNotFoundException ex) {
throw new ScriptNotFoundException("There is no active script for user " + username.asString());
}
- return new File(dir, content);
+ File scriptFile = new File(dir, content);
+ enforceRoot(scriptFile);
+ return scriptFile;
}
protected boolean isActiveFile(Username username, File file) throws StorageException {
@@ -340,6 +357,8 @@ public class SieveFileRepository implements SieveRepository {
File sieveBaseDirectory = personalScriptDirectory.getParentFile();
File activeScriptPersistenceFile = new File(personalScriptDirectory, FILE_NAME_ACTIVE);
File activeScriptCopy = new File(sieveBaseDirectory, userName.asString() + SIEVE_EXTENSION);
+ enforceRoot(activeScriptPersistenceFile);
+ enforceRoot(activeScriptCopy);
if (isActive) {
toFile(activeScriptPersistenceFile, scriptToBeActivated.getName());
try {
@@ -359,6 +378,7 @@ public class SieveFileRepository implements SieveRepository {
protected File getScriptFile(Username username, ScriptName name) throws ScriptNotFoundException, StorageException {
File file = new File(getUserDirectory(username), name.getValue());
+ enforceRoot(file);
if (!file.exists()) {
throw new ScriptNotFoundException("User: " + username + "Script: " + name);
}
diff --git a/server/data/data-file/src/test/java/org/apache/james/sieverepository/file/SieveFileRepositoryTest.java b/server/data/data-file/src/test/java/org/apache/james/sieverepository/file/SieveFileRepositoryTest.java
index 42b9b70..f9afe9f 100644
--- a/server/data/data-file/src/test/java/org/apache/james/sieverepository/file/SieveFileRepositoryTest.java
+++ b/server/data/data-file/src/test/java/org/apache/james/sieverepository/file/SieveFileRepositoryTest.java
@@ -2,17 +2,23 @@
package org.apache.james.sieverepository.file;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import org.apache.commons.io.FileUtils;
+import org.apache.james.core.Username;
import org.apache.james.filesystem.api.FileSystem;
+import org.apache.james.sieverepository.api.ScriptName;
import org.apache.james.sieverepository.api.SieveRepository;
+import org.apache.james.sieverepository.api.exception.StorageException;
import org.apache.james.sieverepository.lib.SieveRepositoryContract;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
class SieveFileRepositoryTest implements SieveRepositoryContract {
@@ -55,4 +61,17 @@ class SieveFileRepositoryTest implements SieveRepositoryContract {
public SieveRepository sieveRepository() {
return sieveRepository;
}
+
+ @Test
+ void putScriptShouldThrowOnCraftedUsername() {
+ assertThatThrownBy(() -> sieveRepository().putScript(Username.of("../../home/interview1/test"), SCRIPT_NAME, SCRIPT_CONTENT))
+ .isInstanceOf(StorageException.class);
+ }
+
+ @Test
+ void putScriptShouldThrowOnCraftedScriptName() {
+ assertThatThrownBy(() -> sieveRepository().putScript(Username.of("test"),
+ new ScriptName("../../../../home/interview1/script"), SCRIPT_CONTENT))
+ .isInstanceOf(StorageException.class);
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org