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:22 UTC
[james-project] 01/02: JAMES-3646 SieveFileRepository should
validate underlying files belong to its root
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