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 <[email protected]> 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: [email protected] For additional commands, e-mail: [email protected]
