From bcf45ee9cf646fd1be4ced0a4004491dff28c7aa Mon Sep 17 00:00:00 2001 From: AssahBismarkabah Date: Thu, 19 Sep 2024 14:43:39 +0100 Subject: [PATCH] fix : path traversal if path contains rootBucket --- .../actions/DefaultInboxActionsModule.java | 3 +- .../impl/service/DefaultDatasafeServices.java | 2 + .../service/VersionedDatasafeServices.java | 2 + .../datasafe/business/impl/e2e/BaseE2EIT.java | 62 ++++++++----------- .../impl/e2e/BasicFunctionalityIT.java | 8 ++- .../impl/e2e/BasicFunctionalityUtf8IT.java | 16 +++-- ...asicFunctionalityWithPasswordChangeIT.java | 8 +-- .../impl/e2e/DataTamperingResistanceIT.java | 12 +--- ...OperationsTestWithDefaultDatasafeTest.java | 16 ++--- 9 files changed, 55 insertions(+), 74 deletions(-) diff --git a/datasafe-business/src/main/java/de/adorsys/datasafe/business/impl/inbox/actions/DefaultInboxActionsModule.java b/datasafe-business/src/main/java/de/adorsys/datasafe/business/impl/inbox/actions/DefaultInboxActionsModule.java index 4cd21a2ed..32bc8411d 100644 --- a/datasafe-business/src/main/java/de/adorsys/datasafe/business/impl/inbox/actions/DefaultInboxActionsModule.java +++ b/datasafe-business/src/main/java/de/adorsys/datasafe/business/impl/inbox/actions/DefaultInboxActionsModule.java @@ -35,7 +35,6 @@ public abstract class DefaultInboxActionsModule { /** * By default, writes file into users' INBOX using his public key (no privatespace access required). */ - @Binds abstract WriteToInbox writeInbox(WriteToInboxImplRuntimeDelegatable impl); @@ -50,4 +49,4 @@ public abstract class DefaultInboxActionsModule { */ @Binds abstract InboxService inboxService(InboxServiceImplRuntimeDelegatable impl); -} +} \ No newline at end of file diff --git a/datasafe-business/src/main/java/de/adorsys/datasafe/business/impl/service/DefaultDatasafeServices.java b/datasafe-business/src/main/java/de/adorsys/datasafe/business/impl/service/DefaultDatasafeServices.java index f8c5e1112..4a0d3a432 100644 --- a/datasafe-business/src/main/java/de/adorsys/datasafe/business/impl/service/DefaultDatasafeServices.java +++ b/datasafe-business/src/main/java/de/adorsys/datasafe/business/impl/service/DefaultDatasafeServices.java @@ -69,6 +69,8 @@ interface Builder { @BindsInstance Builder config(DFSConfig config); + @BindsInstance + Builder rootBucket( String rootBucket); /** * Binds (configures) all storage operations - not necessary to call {@code storageList} after. */ diff --git a/datasafe-business/src/main/java/de/adorsys/datasafe/business/impl/service/VersionedDatasafeServices.java b/datasafe-business/src/main/java/de/adorsys/datasafe/business/impl/service/VersionedDatasafeServices.java index 10769d54b..b750f7d69 100644 --- a/datasafe-business/src/main/java/de/adorsys/datasafe/business/impl/service/VersionedDatasafeServices.java +++ b/datasafe-business/src/main/java/de/adorsys/datasafe/business/impl/service/VersionedDatasafeServices.java @@ -90,6 +90,8 @@ interface Builder { @BindsInstance Builder storage(StorageService storageService); + @BindsInstance + Builder rootBucket( String rootBucket); /** * Provides class overriding functionality, so that you can disable i.e. path encryption * @param overridesRegistry Map with class-overrides (note: you can override classes that are diff --git a/datasafe-business/src/test/java/de/adorsys/datasafe/business/impl/e2e/BaseE2EIT.java b/datasafe-business/src/test/java/de/adorsys/datasafe/business/impl/e2e/BaseE2EIT.java index 77b107a54..c648cfb08 100644 --- a/datasafe-business/src/test/java/de/adorsys/datasafe/business/impl/e2e/BaseE2EIT.java +++ b/datasafe-business/src/test/java/de/adorsys/datasafe/business/impl/e2e/BaseE2EIT.java @@ -43,7 +43,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.stream.Collectors; @@ -120,18 +119,13 @@ protected void writeDataToPrivate(UserIDAuth auth, String path, String data) { @SneakyThrows protected void writeDataToInbox(UserIDAuth owner, UserIDAuth auth, String path, String data) { - String trimmedPath = path.trim(); - String fullPath = "datasafe-root/" + trimmedPath; try (OutputStream stream = writeToInbox.write( - WriteInboxRequest.forDefaultPublic(owner, Collections.singleton(auth.getUserID()), fullPath) + WriteInboxRequest.forDefaultPublic(owner, Collections.singleton(auth.getUserID()), path) )) { - log.info("Writing data '{}' to path '{}' for user '{}'", data, fullPath, auth.getUserID()); + stream.write(data.getBytes(UTF_8)); - log.info("File {} of user {} saved to {}", Obfuscate.secure(data), auth, Obfuscate.secure(fullPath, "/")); - } catch (Exception e) { - log.error("Failed to write data to inbox: {}", e.getMessage()); - throw e; } + log.info("File {} of user {} saved to {}", Obfuscate.secure(data), auth, Obfuscate.secure(path, "/")); } protected AbsoluteLocation getFirstFileInPrivate(UserIDAuth owner) { @@ -140,7 +134,7 @@ protected AbsoluteLocation getFirstFileInPrivate(UserIDAuth ow protected List> getAllFilesInPrivate(UserIDAuth owner) { try (Stream> ls = listPrivate.list( - ListRequest.forDefaultPrivate(owner, "./") + ListRequest.forDefaultPrivate(owner, "./") )) { List> files = ls.collect(Collectors.toList()); log.info("{} has {} in PRIVATE", owner.getUserID(), files); @@ -184,7 +178,7 @@ protected AbsoluteLocation getFirstFileInInbox(UserIDAuth inbo protected List> getAllFilesInInbox(UserIDAuth inboxOwner) { try (Stream> ls = listInbox.list( - ListRequest.forDefaultPrivate(inboxOwner, "./") + ListRequest.forDefaultPrivate(inboxOwner, "./") )) { List> files = ls.collect(Collectors.toList()); log.info("{} has {} in INBOX", inboxOwner, files); @@ -200,7 +194,7 @@ protected void registerJohnAndJane() { @SneakyThrows protected void sendToInbox(UserIDAuth from, UserID to, String filename, String data) { try (OutputStream stream = writeToInbox.write( - WriteInboxRequest.forDefaultPublic(from, Collections.singleton(to), "./" + filename) + WriteInboxRequest.forDefaultPublic(from, Collections.singleton(to), "./" + filename) )) { stream.write(data.getBytes()); } @@ -235,11 +229,11 @@ protected UserIDAuth createJohnTestUser(int i) { protected void assertPrivateSpaceList(UserIDAuth user, String root, String... expected) { List paths; try (Stream> ls = - listPrivate.list(ListRequest.forDefaultPrivate(user, root)) + listPrivate.list(ListRequest.forDefaultPrivate(user, root)) ) { paths = ls - .map(it -> it.getResource().asPrivate().decryptedPath().asString()) - .collect(Collectors.toList()); + .map(it -> it.getResource().asPrivate().decryptedPath().asString()) + .collect(Collectors.toList()); } assertThat(paths).containsExactlyInAnyOrder(expected); @@ -247,28 +241,22 @@ protected void assertPrivateSpaceList(UserIDAuth user, String root, String... ex protected void assertInboxSpaceList(UserIDAuth user, String root, String... expected) { List paths; - String prefixedRoot = "datasafe-root/" + root; - try (Stream> ls = - listInbox.list(ListRequest.forDefaultPrivate(user, prefixedRoot)) + listInbox.list(ListRequest.forDefaultPrivate(user, root)) ) { paths = ls .map(it -> it.getResource().asPrivate().decryptedPath().asString()) .collect(Collectors.toList()); } - String[] prefixedExpected = Arrays.stream(expected) - .map(path -> "datasafe-root/" + path) - .toArray(String[]::new); - - assertThat(paths).containsExactlyInAnyOrder(prefixedExpected); + assertThat(paths).containsExactlyInAnyOrder(expected); } @SneakyThrows protected void assertRootDirIsEmpty(WithStorageProvider.StorageDescriptor descriptor) { try (Stream> ls = descriptor.getStorageService().get() - .list( - new AbsoluteLocation<>(BasePrivateResource.forPrivate(descriptor.getLocation()))) + .list( + new AbsoluteLocation<>(BasePrivateResource.forPrivate(descriptor.getLocation()))) ) { assertThat(ls).isEmpty(); } @@ -279,18 +267,18 @@ protected void assertRootDirIsEmpty(WithStorageProvider.StorageDescriptor descri // however we can't remove anything above try (Stream files = Files.walk(Paths.get(descriptor.getLocation().asURI()))) { assertThat(files) - .allMatch(it -> it.toFile().isDirectory()) - .extracting(Path::toUri) - .extracting(it -> descriptor.getLocation().asURI().relativize(it)) - .extracting(URI::toString) - .containsExactlyInAnyOrder( - "", - "users/", - "profiles/", - "profiles/public/", - "profiles/private/" - ); + .allMatch(it -> it.toFile().isDirectory()) + .extracting(Path::toUri) + .extracting(it -> descriptor.getLocation().asURI().relativize(it)) + .extracting(URI::toString) + .containsExactlyInAnyOrder( + "", + "users/", + "profiles/", + "profiles/public/", + "profiles/private/" + ); } } } -} +} \ No newline at end of file diff --git a/datasafe-business/src/test/java/de/adorsys/datasafe/business/impl/e2e/BasicFunctionalityIT.java b/datasafe-business/src/test/java/de/adorsys/datasafe/business/impl/e2e/BasicFunctionalityIT.java index 51cbd9f95..07cfde934 100644 --- a/datasafe-business/src/test/java/de/adorsys/datasafe/business/impl/e2e/BasicFunctionalityIT.java +++ b/datasafe-business/src/test/java/de/adorsys/datasafe/business/impl/e2e/BasicFunctionalityIT.java @@ -203,11 +203,12 @@ void testMultipleRecipientsSharing(WithStorageProvider.StorageDescriptor descrip init(descriptor); UserIDAuth owner = registerUser("owner"); + UserIDAuth john = registerUser("john"); UserIDAuth jane = registerUser("jane"); UserIDAuth jamie = registerUser("jamie"); - String multiShareFile = "datasafe-root/multishare.txt"; // Corrected path with prefix + String multiShareFile = "multishare.txt"; try (OutputStream os = writeToInbox.write(WriteInboxRequest.forDefaultPublic( owner, ImmutableSet.of(john.getUserID(), jane.getUserID(), jamie.getUserID()), @@ -228,11 +229,12 @@ void testMultipleRecipientsSharingLargeChunk(WithStorageProvider.StorageDescript init(descriptor); UserIDAuth owner = registerUser("owner"); + UserIDAuth john = registerUser("john"); UserIDAuth jane = registerUser("jane"); UserIDAuth jamie = registerUser("jamie"); - String multiShareFile = "datasafe-root/multishare.txt"; // Corrected path with prefix + String multiShareFile = "multishare.txt"; byte[] bytes = new byte[LARGE_SIZE]; ThreadLocalRandom.current().nextBytes(bytes); try (OutputStream os = writeToInbox.write(WriteInboxRequest.forDefaultPublic( @@ -447,4 +449,4 @@ private static ObjectMapper createMapper() { mapper.setSerializationInclusion(JsonInclude.Include.NON_NULL); return mapper; } -} +} \ No newline at end of file diff --git a/datasafe-business/src/test/java/de/adorsys/datasafe/business/impl/e2e/BasicFunctionalityUtf8IT.java b/datasafe-business/src/test/java/de/adorsys/datasafe/business/impl/e2e/BasicFunctionalityUtf8IT.java index 4e324f3f6..bda2682af 100644 --- a/datasafe-business/src/test/java/de/adorsys/datasafe/business/impl/e2e/BasicFunctionalityUtf8IT.java +++ b/datasafe-business/src/test/java/de/adorsys/datasafe/business/impl/e2e/BasicFunctionalityUtf8IT.java @@ -89,13 +89,13 @@ void readInboxContentWithUnicodeUsingUnicodePath(WithStorageProvider.StorageDesc john = registerUser("john"); jane = registerUser("jane"); + String unicodeMessage = "привет мир!"; - String path = " привет/prüfungsdokument=/файл:&? с пробелом.док"; - writeDataToInbox(john, jane, path, unicodeMessage); + writeDataToInbox(john, jane, " привет/prüfungsdokument=/файл:&? с пробелом.док", unicodeMessage); String inboxContentJane = readInboxUsingPrivateKey( jane, - BasePrivateResource.forPrivate("datasafe-root/" + path.trim()) + BasePrivateResource.forPrivate(" привет/prüfungsdokument=/файл:&? с пробелом.док") ); assertThat(inboxContentJane).isEqualTo(unicodeMessage); @@ -108,10 +108,9 @@ void listingInboxPathWithUnicode(WithStorageProvider.StorageDescriptor descripto registerJohnAndJane(); - //String rootBucket = provideRootBucket(); - writeDataToInbox(john, jane, "/prüfungsdokument.doc+doc", MESSAGE_ONE); - writeDataToInbox(john, jane, "/уровень1/?файл+doc", MESSAGE_ONE); - writeDataToInbox(john, jane, "/уровень1/уровень 2=+/&файл пробел+плюс", MESSAGE_ONE); + writeDataToInbox(john, jane, "prüfungsdokument.doc+doc", MESSAGE_ONE); + writeDataToInbox(john, jane, "уровень1/?файл+doc", MESSAGE_ONE); + writeDataToInbox(john, jane, "уровень1/уровень 2=+/&файл пробел+плюс", MESSAGE_ONE); assertInboxSpaceList(jane, "", "prüfungsdokument.doc+doc", "уровень1/?файл+doc", "уровень1/уровень 2=+/&файл пробел+плюс"); assertInboxSpaceList(jane, "./", "prüfungsdokument.doc+doc", "уровень1/?файл+doc", "уровень1/уровень 2=+/&файл пробел+плюс"); @@ -126,10 +125,9 @@ void listingInboxPathWithUnicode(WithStorageProvider.StorageDescriptor descripto assertInboxSpaceList(jane, "./уровень1/уровень 2=+/", "уровень1/уровень 2=+/&файл пробел+плюс"); } - private void init(StorageDescriptor descriptor) { DefaultDatasafeServices datasafeServices = DatasafeServicesProvider .defaultDatasafeServices(descriptor.getStorageService().get(), descriptor.getLocation()); initialize(DatasafeServicesProvider.dfsConfig(descriptor.getLocation()), datasafeServices); } -} +} \ No newline at end of file diff --git a/datasafe-business/src/test/java/de/adorsys/datasafe/business/impl/e2e/BasicFunctionalityWithPasswordChangeIT.java b/datasafe-business/src/test/java/de/adorsys/datasafe/business/impl/e2e/BasicFunctionalityWithPasswordChangeIT.java index 71329abf8..6f244dca9 100644 --- a/datasafe-business/src/test/java/de/adorsys/datasafe/business/impl/e2e/BasicFunctionalityWithPasswordChangeIT.java +++ b/datasafe-business/src/test/java/de/adorsys/datasafe/business/impl/e2e/BasicFunctionalityWithPasswordChangeIT.java @@ -25,7 +25,6 @@ import static de.adorsys.datasafe.business.impl.e2e.Const.MESSAGE_ONE; import static de.adorsys.datasafe.business.impl.e2e.Const.PRIVATE_FILE_PATH; import static de.adorsys.datasafe.business.impl.e2e.Const.SHARED_FILE_PATH; -import static de.adorsys.datasafe.business.impl.inbox.actions.DefaultInboxActionsModule_ProvideRootBucketFactory.provideRootBucket; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -67,9 +66,7 @@ void testMultipleRecipientsSharing(WithStorageProvider.StorageDescriptor descrip UserIDAuth jane = registerUser("jane"); UserIDAuth jamie = registerUser("jamie"); - String rootBucket = provideRootBucket(); // Added - String multiShareFile = rootBucket + "/multishare.txt"; // Updated - + String multiShareFile = "multishare.txt"; multishareFiles(sender, john, jane, jamie, multiShareFile); Stream.of(john, jane, jamie).forEach( @@ -81,7 +78,6 @@ void testMultipleRecipientsSharing(WithStorageProvider.StorageDescriptor descrip ); } - @ParameterizedTest @MethodSource("allStorages") void testWriteToPrivateListPrivateReadPrivateAndSendToAndReadFromInbox( @@ -219,4 +215,4 @@ private void init(WithStorageProvider.StorageDescriptor descriptor) { .defaultDatasafeServices(descriptor.getStorageService().get(), descriptor.getLocation()); initialize(DatasafeServicesProvider.dfsConfig(descriptor.getLocation()), datasafeServices); } -} +} \ No newline at end of file diff --git a/datasafe-business/src/test/java/de/adorsys/datasafe/business/impl/e2e/DataTamperingResistanceIT.java b/datasafe-business/src/test/java/de/adorsys/datasafe/business/impl/e2e/DataTamperingResistanceIT.java index d4feed792..526f0376b 100644 --- a/datasafe-business/src/test/java/de/adorsys/datasafe/business/impl/e2e/DataTamperingResistanceIT.java +++ b/datasafe-business/src/test/java/de/adorsys/datasafe/business/impl/e2e/DataTamperingResistanceIT.java @@ -31,7 +31,6 @@ import java.util.concurrent.ThreadLocalRandom; import java.util.stream.Stream; -import static de.adorsys.datasafe.business.impl.inbox.actions.DefaultInboxActionsModule_ProvideRootBucketFactory.provideRootBucket; import static org.assertj.core.api.Java6Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -80,15 +79,11 @@ void testPrivateDocumentContentTamperResistance() { } } - // Updated DataTamperingResistanceIT.java @Test @SneakyThrows void testInboxDocumentContentTamperResistance() { - String rootBucket = provideRootBucket(); - String fullFileName = rootBucket + "/" + FILENAME; - try (OutputStream os = writeToInbox.write( - WriteInboxRequest.forDefaultPublic(jane, Collections.singleton(john.getUserID()), fullFileName)) + WriteInboxRequest.forDefaultPublic(jane, Collections.singleton(john.getUserID()), FILENAME)) ) { os.write(FILE_TEXT.getBytes(StandardCharsets.UTF_8)); } @@ -96,14 +91,13 @@ void testInboxDocumentContentTamperResistance() { AbsoluteLocation inboxFile = getFirstFileInInbox(john); tamperFileByReplacingOneByteOfEncryptedMessage(inboxFile); - try (InputStream is = readFromInbox.read(ReadRequest.forDefaultPrivate(john, fullFileName))) { + try (InputStream is = readFromInbox.read(ReadRequest.forDefaultPrivate(john, FILENAME))) { assertThatThrownBy( () -> ByteStreams.copy(is, ByteStreams.nullOutputStream()) ).isInstanceOf(InvalidCipherTextIOException.class).hasCauseInstanceOf(AEADBadTagException.class); } } - @ParameterizedTest(name = "{arguments}") @ValueSource(strings = {FILENAME, DIR_AND_FILENAME, DIR_DIR_AND_FILENAME}) @SneakyThrows @@ -185,4 +179,4 @@ private byte randomByte(byte notEqualTo) { return resultArray[0]; } -} +} \ No newline at end of file diff --git a/datasafe-examples/datasafe-examples-business/src/test/java/de/adorsys/datasafe/examples/business/filesystem/BaseUserOperationsTestWithDefaultDatasafeTest.java b/datasafe-examples/datasafe-examples-business/src/test/java/de/adorsys/datasafe/examples/business/filesystem/BaseUserOperationsTestWithDefaultDatasafeTest.java index ddaa12925..ccdfeec76 100644 --- a/datasafe-examples/datasafe-examples-business/src/test/java/de/adorsys/datasafe/examples/business/filesystem/BaseUserOperationsTestWithDefaultDatasafeTest.java +++ b/datasafe-examples/datasafe-examples-business/src/test/java/de/adorsys/datasafe/examples/business/filesystem/BaseUserOperationsTestWithDefaultDatasafeTest.java @@ -136,7 +136,7 @@ void shareWithJane() { } // END_SNIPPET - assertThat(defaultDatasafeServices.inboxService().read(ReadRequest.forDefaultPrivate(jane, "datasafe-root/hello.txt"))) + assertThat(defaultDatasafeServices.inboxService().read(ReadRequest.forDefaultPrivate(jane, "hello.txt"))) .hasContent("Hello Jane"); } @@ -162,9 +162,9 @@ void shareWithJaneAndJamie() { } // END_SNIPPET - assertThat(defaultDatasafeServices.inboxService().read(ReadRequest.forDefaultPrivate(jane, "datasafe-root/hello.txt"))) + assertThat(defaultDatasafeServices.inboxService().read(ReadRequest.forDefaultPrivate(jane, "hello.txt"))) .hasContent("Hello Jane and Jamie"); - assertThat(defaultDatasafeServices.inboxService().read(ReadRequest.forDefaultPrivate(jamie, "datasafe-root/hello.txt"))) + assertThat(defaultDatasafeServices.inboxService().read(ReadRequest.forDefaultPrivate(jamie, "hello.txt"))) .hasContent("Hello Jane and Jamie"); } @@ -230,18 +230,18 @@ void listInbox() { assertThat(johnsInboxFilesInRoot) .extracting(it -> it.getResource().asPrivate().decryptedPath().toASCIIString()) .containsExactlyInAnyOrder( - "datasafe-root/home/my/secret.txt", - "datasafe-root/home/watch/films.txt" + "home/my/secret.txt", + "home/watch/films.txt" ); // Now let's list John's home/watch: List> johnsInboxFilesInWatch = defaultDatasafeServices.inboxService() - .list(ListRequest.forDefaultPrivate(user, "datasafe-root/home/watch")).collect(Collectors.toList()); + .list(ListRequest.forDefaultPrivate(user, "home/watch")).collect(Collectors.toList()); // same files we created assertThat(johnsInboxFilesInWatch) .extracting(it -> it.getResource().asPrivate().decryptedPath().toASCIIString()) .containsExactly( - "datasafe-root/home/watch/films.txt" + "home/watch/films.txt" ); // END_SNIPPET } @@ -316,4 +316,4 @@ private void shareMessage(UserIDAuth fromUser, UserID forUser, String messageNam os.write(message.getBytes(StandardCharsets.UTF_8)); } } -} +} \ No newline at end of file