Skip to content

Commit

Permalink
Fix: Ensure paths are securely prefixed with root bucket to prevent a…
Browse files Browse the repository at this point in the history
…rbitrary file write
  • Loading branch information
AssahBismarkabah committed Aug 8, 2024
1 parent ea27fc6 commit 0f5a4f4
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

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

/**
Expand All @@ -47,10 +51,16 @@ public WriteToInboxImpl(PublicKeyService publicKeyService, PrivateKeyService pri
*/
@Override
public OutputStream write(WriteInboxRequest<UserIDAuth, Set<UserID>, 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())
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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());
}
Expand Down

0 comments on commit 0f5a4f4

Please sign in to comment.