diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/DirectDownloadCallable.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/DirectDownloadCallable.java index f2c8ba1776..3b21a80cd9 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/DirectDownloadCallable.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/DirectDownloadCallable.java @@ -36,27 +36,30 @@ final class DirectDownloadCallable implements Callable { private final Storage.BlobSourceOption[] opts; + private final Path destPath; + DirectDownloadCallable( Storage storage, BlobInfo originalBlob, ParallelDownloadConfig parallelDownloadConfig, - BlobSourceOption[] opts) { + BlobSourceOption[] opts, + Path destPath) { this.originalBlob = originalBlob; this.parallelDownloadConfig = parallelDownloadConfig; this.storage = storage; this.opts = opts; + this.destPath = destPath; } @Override public DownloadResult call() { - Path path = TransferManagerUtils.createDestPath(parallelDownloadConfig, originalBlob); long bytesCopied = -1L; try (ReadChannel rc = storage.reader( BlobId.of(parallelDownloadConfig.getBucketName(), originalBlob.getName()), opts); FileChannel wc = FileChannel.open( - path, + destPath, StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING)) { @@ -89,7 +92,7 @@ public DownloadResult call() { } DownloadResult result = DownloadResult.newBuilder(originalBlob, TransferStatus.SUCCESS) - .setOutputDestination(path.toAbsolutePath()) + .setOutputDestination(destPath.toAbsolutePath()) .build(); return result; } diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/PathTraversalBlockedException.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/PathTraversalBlockedException.java new file mode 100644 index 0000000000..78d733e809 --- /dev/null +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/PathTraversalBlockedException.java @@ -0,0 +1,36 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.storage.transfermanager; + +import java.nio.file.Path; +import java.util.Locale; + +/** + * Exception thrown when a download is blocked because the object name would result in a path + * traversal outside the target directory. + */ +public final class PathTraversalBlockedException extends RuntimeException { + + public PathTraversalBlockedException(String objectName, Path targetDirectory) { + super( + String.format( + Locale.US, + "Download of object '%s' was blocked because it would escape the target directory '%s'.", + objectName, + targetDirectory)); + } +} diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/TransferManagerImpl.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/TransferManagerImpl.java index d005441924..aa1cfadf1e 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/TransferManagerImpl.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/TransferManagerImpl.java @@ -145,15 +145,20 @@ public void close() throws Exception { Storage.BlobSourceOption[] opts = config.getOptionsPerRequest().toArray(new Storage.BlobSourceOption[0]); List> downloadTasks = new ArrayList<>(); - if (!transferManagerConfig.isAllowDivideAndConquerDownload()) { - for (BlobInfo blob : blobs) { - DirectDownloadCallable callable = new DirectDownloadCallable(storage, blob, config, opts); - downloadTasks.add(convert(executor.submit(callable))); + for (BlobInfo blob : blobs) { + Path destPath = TransferManagerUtils.createAndValidateDestPath(config, blob); + if (destPath == null) { + DownloadResult skipped = + DownloadResult.newBuilder(blob, TransferStatus.FAILED_TO_START) + .setException( + new PathTraversalBlockedException( + blob.getName(), config.getDownloadDirectory())) + .build(); + downloadTasks.add(ApiFutures.immediateFuture(skipped)); + continue; } - } else { - for (BlobInfo blob : blobs) { + if (transferManagerConfig.isAllowDivideAndConquerDownload()) { BlobInfo validatedBlob = retrieveSizeAndGeneration(storage, blob, config.getBucketName()); - Path destPath = TransferManagerUtils.createDestPath(config, blob); if (validatedBlob != null && qos.divideAndConquer(validatedBlob.getSize())) { DownloadResult optimisticResult = DownloadResult.newBuilder(validatedBlob, TransferStatus.SUCCESS) @@ -181,12 +186,14 @@ public void close() throws Exception { DownloadSegment::reduce, BinaryOperator.minBy(DownloadResult.COMPARATOR)), MoreExecutors.directExecutor())); - } else { - DirectDownloadCallable callable = new DirectDownloadCallable(storage, blob, config, opts); - downloadTasks.add(convert(executor.submit(callable))); + continue; } } + DirectDownloadCallable callable = + new DirectDownloadCallable(storage, blob, config, opts, destPath); + downloadTasks.add(convert(executor.submit(callable))); } + return DownloadJob.newBuilder() .setDownloadResults(downloadTasks) .setParallelDownloadConfig(config) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/TransferManagerUtils.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/TransferManagerUtils.java index a884aa7067..5a9103982a 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/TransferManagerUtils.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/TransferManagerUtils.java @@ -26,11 +26,21 @@ final class TransferManagerUtils { private TransferManagerUtils() {} - static Path createDestPath(ParallelDownloadConfig config, BlobInfo originalBlob) { + static Path createAndValidateDestPath(ParallelDownloadConfig config, BlobInfo originalBlob) { Path newPath = config .getDownloadDirectory() - .resolve(originalBlob.getName().replaceFirst(config.getStripPrefix(), "")); + .resolve(originalBlob.getName().replaceFirst(config.getStripPrefix(), "")) + .toAbsolutePath() + .normalize(); + + Path targetDirectory = config.getDownloadDirectory().toAbsolutePath().normalize(); + + // Security check: Verify the resolved path is inside the target directory + // This catches ".." sequences that attempt to "escape" the folder. + if (!newPath.startsWith(targetDirectory)) { + return null; + } // Check to make sure the parent directories exist if (Files.exists(newPath.getParent())) { return newPath; diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITTransferManagerTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITTransferManagerTest.java index dd7708b5f9..43ad609739 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITTransferManagerTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITTransferManagerTest.java @@ -41,6 +41,7 @@ import com.google.cloud.storage.transfermanager.DownloadResult; import com.google.cloud.storage.transfermanager.ParallelDownloadConfig; import com.google.cloud.storage.transfermanager.ParallelUploadConfig; +import com.google.cloud.storage.transfermanager.PathTraversalBlockedException; import com.google.cloud.storage.transfermanager.TransferManager; import com.google.cloud.storage.transfermanager.TransferManagerConfig; import com.google.cloud.storage.transfermanager.TransferManagerConfigTestingInstances; @@ -635,6 +636,87 @@ public void bucketNameFromUploadBlobInfoFactoryMustMatchConfig() throws Exceptio } } + @Test + public void downloadBlobsPathTraversalBlocked() throws Exception { + TransferManagerConfig config = + TransferManagerConfigTestingInstances.defaults(storage.getOptions()); + try (TransferManager transferManager = config.getService()) { + String bucketName = bucket.getName(); + // Create an object with a name that attempts to "escape" the target directory + String maliciousName = "../malicious.txt"; + BlobInfo maliciousBlob = BlobInfo.newBuilder(BlobId.of(bucketName, maliciousName)).build(); + storage.create( + maliciousBlob, "malicious content".getBytes(java.nio.charset.StandardCharsets.UTF_8)); + + ParallelDownloadConfig parallelDownloadConfig = + ParallelDownloadConfig.newBuilder() + .setBucketName(bucketName) + .setDownloadDirectory(baseDir) // baseDir is the target + .build(); + + List blobsToDownload = new ArrayList<>(blobs); + blobsToDownload.add(maliciousBlob); + + DownloadJob job = transferManager.downloadBlobs(blobsToDownload, parallelDownloadConfig); + List results = job.getDownloadResults(); + + try { + long successCount = + results.stream().filter(res -> res.getStatus() == TransferStatus.SUCCESS).count(); + assertThat(successCount).isEqualTo(blobs.size()); + + // Verify that the malicious blob was blocked/skipped + Optional blockedResult = + results.stream() + .filter(res -> res.getInput().getName().equals(maliciousName)) + .findFirst(); + + assertThat(blockedResult.isPresent()).isTrue(); + assertThat(blockedResult.get().getStatus()).isEqualTo(TransferStatus.FAILED_TO_START); + assertThat(blockedResult.get().getException()) + .isInstanceOf(PathTraversalBlockedException.class); + assertThat(blockedResult.get().getException().getMessage()).contains("blocked"); + } finally { + storage.delete(maliciousBlob.getBlobId()); + } + } + } + + @Test + public void downloadBlobsPathTraversalAllowedWithinTarget() throws Exception { + TransferManagerConfig config = + TransferManagerConfigTestingInstances.defaults(storage.getOptions()); + try (TransferManager transferManager = config.getService()) { + String bucketName = bucket.getName(); + // This name resolves to 'safe.txt' inside the target directory + String safeNameWithDots = "subdir/../safe.txt"; + BlobInfo safeBlob = BlobInfo.newBuilder(BlobId.of(bucketName, safeNameWithDots)).build(); + storage.create(safeBlob, "safe content".getBytes(java.nio.charset.StandardCharsets.UTF_8)); + + ParallelDownloadConfig parallelDownloadConfig = + ParallelDownloadConfig.newBuilder() + .setBucketName(bucketName) + .setDownloadDirectory(baseDir) + .build(); + + DownloadJob job = + transferManager.downloadBlobs( + Collections.singletonList(safeBlob), parallelDownloadConfig); + List results = job.getDownloadResults(); + + try { + assertThat(results.get(0).getStatus()).isEqualTo(TransferStatus.SUCCESS); + // Verify it was saved to the correct normalized location + Path expectedPath = baseDir.resolve("safe.txt").toAbsolutePath().normalize(); + assertThat(results.get(0).getOutputDestination().toAbsolutePath().normalize()) + .isEqualTo(expectedPath); + } finally { + cleanUpFiles(results); + storage.delete(safeBlob.getBlobId()); + } + } + } + private void cleanUpFiles(List results) throws IOException { // Cleanup downloaded blobs and the parent directory for (DownloadResult res : results) {