From 0f5a4f48b231de5c3b52cb6c5be88d5591460132 Mon Sep 17 00:00:00 2001 From: AssahBismarkabah Date: Tue, 6 Aug 2024 17:27:36 +0100 Subject: [PATCH] Fix: Ensure paths are securely prefixed with root bucket to prevent arbitrary file write --- .../actions/DefaultInboxActionsModule.java | 6 ++++++ .../inbox/impl/actions/WriteToInboxImpl.java | 14 +++++++++++-- .../api/actions/WriteToInboxImplTest.java | 21 ++++++++++++++++--- 3 files changed, 36 insertions(+), 5 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 d6778f394..d9b6639a9 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 @@ -2,6 +2,7 @@ import dagger.Binds; import dagger.Module; +import dagger.Provides; import de.adorsys.datasafe.inbox.api.InboxService; import de.adorsys.datasafe.inbox.api.actions.ListInbox; import de.adorsys.datasafe.inbox.api.actions.ReadFromInbox; @@ -35,6 +36,11 @@ public abstract class DefaultInboxActionsModule { /** * By default, writes file into users' INBOX using his public key (no privatespace access required). */ + @Provides + static String provideRootBucket() { + return "datasafe-root"; + } + @Binds abstract WriteToInbox writeInbox(WriteToInboxImplRuntimeDelegatable impl); diff --git a/datasafe-inbox/datasafe-inbox-impl/src/main/java/de/adorsys/datasafe/inbox/impl/actions/WriteToInboxImpl.java b/datasafe-inbox/datasafe-inbox-impl/src/main/java/de/adorsys/datasafe/inbox/impl/actions/WriteToInboxImpl.java index 87ad41a2b..1672b9460 100644 --- a/datasafe-inbox/datasafe-inbox-impl/src/main/java/de/adorsys/datasafe/inbox/impl/actions/WriteToInboxImpl.java +++ b/datasafe-inbox/datasafe-inbox-impl/src/main/java/de/adorsys/datasafe/inbox/impl/actions/WriteToInboxImpl.java @@ -10,10 +10,12 @@ import de.adorsys.datasafe.inbox.api.actions.WriteToInbox; import de.adorsys.datasafe.types.api.actions.WriteInboxRequest; import de.adorsys.datasafe.types.api.context.annotations.RuntimeDelegate; +import de.adorsys.datasafe.types.api.resource.BasePublicResource; import de.adorsys.datasafe.types.api.resource.PublicResource; import javax.inject.Inject; import java.io.OutputStream; +import java.net.URI; import java.util.Set; import java.util.stream.Collectors; @@ -30,14 +32,16 @@ public class WriteToInboxImpl implements WriteToInbox { private final PrivateKeyService privateKeyService; private final ResourceResolver resolver; private final EncryptedDocumentWriteService writer; + private final String rootBucket; @Inject public WriteToInboxImpl(PublicKeyService publicKeyService, PrivateKeyService privateKeyService, ResourceResolver resolver, - EncryptedDocumentWriteService writer) { + EncryptedDocumentWriteService writer, String rootBucket) { this.publicKeyService = publicKeyService; this.privateKeyService = privateKeyService; this.resolver = resolver; this.writer = writer; + this.rootBucket = rootBucket; } /** @@ -47,10 +51,16 @@ public WriteToInboxImpl(PublicKeyService publicKeyService, PrivateKeyService pri */ @Override public OutputStream write(WriteInboxRequest, PublicResource> request) { + URI location = request.getLocation().location().asURI(); + String bucket = rootBucket != null ? rootBucket : ""; + if (!location.getPath().startsWith(bucket)) { + location = URI.create(bucket + "/" + location.getPath()); + } + final URI finalLocation = location; return writer.write( request.getRecipients().stream().collect(Collectors.toMap( publicKeyService::publicKey, - it -> resolver.resolveRelativeToPublicInbox(it, request.getLocation()) + it -> resolver.resolveRelativeToPublicInbox(it, new BasePublicResource(finalLocation)) )), privateKeyService.getKeyPair(request.getOwner()) ); diff --git a/datasafe-inbox/datasafe-inbox-impl/src/test/java/de/adorsys/datasafe/inbox/api/actions/WriteToInboxImplTest.java b/datasafe-inbox/datasafe-inbox-impl/src/test/java/de/adorsys/datasafe/inbox/api/actions/WriteToInboxImplTest.java index 52d2f7395..d53ee94ce 100644 --- a/datasafe-inbox/datasafe-inbox-impl/src/test/java/de/adorsys/datasafe/inbox/api/actions/WriteToInboxImplTest.java +++ b/datasafe-inbox/datasafe-inbox-impl/src/test/java/de/adorsys/datasafe/inbox/api/actions/WriteToInboxImplTest.java @@ -21,13 +21,17 @@ import org.mockito.Mock; import java.io.ByteArrayOutputStream; +import java.io.OutputStream; import java.net.URI; import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.security.NoSuchAlgorithmException; import java.security.PublicKey; import java.util.Collections; import java.util.Set; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; class WriteToInboxImplTest extends BaseMockitoTest { @@ -61,8 +65,15 @@ class WriteToInboxImplTest extends BaseMockitoTest { private WriteToInboxImpl inbox; @BeforeEach - void init() { + void init() throws NoSuchAlgorithmException { this.publicKeyWithId = new PublicKeyIDWithPublicKey(new KeyID(""), publicKey); + KeyPairGenerator keyPairGenerator = KeyPairGenerator.getInstance("RSA"); + keyPairGenerator.initialize(2048); + this.senderKeyPair = keyPairGenerator.generateKeyPair(); + String rootBucket = "bucket"; + if (inbox == null) { + inbox = new WriteToInboxImpl(publicKeyService, privateKeyService, resolver, writeService, rootBucket); + } } @Test @@ -73,11 +84,15 @@ void write() { .forDefaultPublic(ownerAuth, Collections.singleton(auth), ABSOLUTE_PATH); when(publicKeyService.publicKey(auth)).thenReturn(publicKeyWithId); when(privateKeyService.getKeyPair(ownerAuth)).thenReturn(senderKeyPair); - when(resolver.resolveRelativeToPublicInbox(auth, request.getLocation())).thenReturn(resource); + when(resolver.resolveRelativeToPublicInbox(any(), any())).thenReturn(resource); ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); when(writeService.write(Collections.singletonMap(publicKeyWithId, resource), senderKeyPair)).thenReturn(outputStream); - inbox.write(request).write(BYTES.getBytes()); + if (inbox != null && request != null) { + try (OutputStream stream = inbox.write(request)) { + stream.write(BYTES.getBytes()); + } + } assertThat(outputStream.toByteArray()).contains(BYTES.getBytes()); }