Skip to content

Commit

Permalink
fix : path traversal if path contains rootBucket
Browse files Browse the repository at this point in the history
  • Loading branch information
AssahBismarkabah committed Sep 19, 2024
1 parent 3a13233 commit bcf45ee
Show file tree
Hide file tree
Showing 9 changed files with 55 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -50,4 +49,4 @@ public abstract class DefaultInboxActionsModule {
*/
@Binds
abstract InboxService inboxService(InboxServiceImplRuntimeDelegatable impl);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<ResolvedResource> getFirstFileInPrivate(UserIDAuth owner) {
Expand All @@ -140,7 +134,7 @@ protected AbsoluteLocation<ResolvedResource> getFirstFileInPrivate(UserIDAuth ow

protected List<AbsoluteLocation<ResolvedResource>> getAllFilesInPrivate(UserIDAuth owner) {
try (Stream<AbsoluteLocation<ResolvedResource>> ls = listPrivate.list(
ListRequest.forDefaultPrivate(owner, "./")
ListRequest.forDefaultPrivate(owner, "./")
)) {
List<AbsoluteLocation<ResolvedResource>> files = ls.collect(Collectors.toList());
log.info("{} has {} in PRIVATE", owner.getUserID(), files);
Expand Down Expand Up @@ -184,7 +178,7 @@ protected AbsoluteLocation<ResolvedResource> getFirstFileInInbox(UserIDAuth inbo

protected List<AbsoluteLocation<ResolvedResource>> getAllFilesInInbox(UserIDAuth inboxOwner) {
try (Stream<AbsoluteLocation<ResolvedResource>> ls = listInbox.list(
ListRequest.forDefaultPrivate(inboxOwner, "./")
ListRequest.forDefaultPrivate(inboxOwner, "./")
)) {
List<AbsoluteLocation<ResolvedResource>> files = ls.collect(Collectors.toList());
log.info("{} has {} in INBOX", inboxOwner, files);
Expand All @@ -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());
}
Expand Down Expand Up @@ -235,40 +229,34 @@ protected UserIDAuth createJohnTestUser(int i) {
protected void assertPrivateSpaceList(UserIDAuth user, String root, String... expected) {
List<String> paths;
try (Stream<AbsoluteLocation<ResolvedResource>> 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);
}

protected void assertInboxSpaceList(UserIDAuth user, String root, String... expected) {
List<String> paths;
String prefixedRoot = "datasafe-root/" + root;

try (Stream<AbsoluteLocation<ResolvedResource>> 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<AbsoluteLocation<ResolvedResource>> ls = descriptor.getStorageService().get()
.list(
new AbsoluteLocation<>(BasePrivateResource.forPrivate(descriptor.getLocation())))
.list(
new AbsoluteLocation<>(BasePrivateResource.forPrivate(descriptor.getLocation())))
) {
assertThat(ls).isEmpty();
}
Expand All @@ -279,18 +267,18 @@ protected void assertRootDirIsEmpty(WithStorageProvider.StorageDescriptor descri
// however we can't remove anything above
try (Stream<Path> 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/"
);
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand All @@ -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(
Expand Down Expand Up @@ -447,4 +449,4 @@ private static ObjectMapper createMapper() {
mapper.setSerializationInclusion(JsonInclude.Include.NON_NULL);
return mapper;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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=+/&файл пробел+плюс");
Expand All @@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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(
Expand All @@ -81,7 +78,6 @@ void testMultipleRecipientsSharing(WithStorageProvider.StorageDescriptor descrip
);
}


@ParameterizedTest
@MethodSource("allStorages")
void testWriteToPrivateListPrivateReadPrivateAndSendToAndReadFromInbox(
Expand Down Expand Up @@ -219,4 +215,4 @@ private void init(WithStorageProvider.StorageDescriptor descriptor) {
.defaultDatasafeServices(descriptor.getStorageService().get(), descriptor.getLocation());
initialize(DatasafeServicesProvider.dfsConfig(descriptor.getLocation()), datasafeServices);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -80,30 +79,25 @@ 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));
}

AbsoluteLocation<ResolvedResource> 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
Expand Down Expand Up @@ -185,4 +179,4 @@ private byte randomByte(byte notEqualTo) {

return resultArray[0];
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}

Expand All @@ -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");
}

Expand Down Expand Up @@ -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<AbsoluteLocation<ResolvedResource>> 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
}
Expand Down Expand Up @@ -316,4 +316,4 @@ private void shareMessage(UserIDAuth fromUser, UserID forUser, String messageNam
os.write(message.getBytes(StandardCharsets.UTF_8));
}
}
}
}

0 comments on commit bcf45ee

Please sign in to comment.