Skip to content

Commit 49abf75

Browse files
nidhiii-27nidhiii-27
authored
fix: validate blob paths to prevent directory traversal in TransferManager downloads (#3455)
1 parent 3c4e96d commit 49abf75

6 files changed

Lines changed: 158 additions & 19 deletions

File tree

‎google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/DirectDownloadCallable.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,27 +36,30 @@ final class DirectDownloadCallable implements Callable<DownloadResult> {
3636

3737
private final Storage.BlobSourceOption[] opts;
3838

39+
private final Path destPath;
40+
3941
DirectDownloadCallable(
4042
Storage storage,
4143
BlobInfo originalBlob,
4244
ParallelDownloadConfig parallelDownloadConfig,
43-
BlobSourceOption[] opts) {
45+
BlobSourceOption[] opts,
46+
Path destPath) {
4447
this.originalBlob = originalBlob;
4548
this.parallelDownloadConfig = parallelDownloadConfig;
4649
this.storage = storage;
4750
this.opts = opts;
51+
this.destPath = destPath;
4852
}
4953

5054
@Override
5155
public DownloadResult call() {
52-
Path path = TransferManagerUtils.createDestPath(parallelDownloadConfig, originalBlob);
5356
long bytesCopied = -1L;
5457
try (ReadChannel rc =
5558
storage.reader(
5659
BlobId.of(parallelDownloadConfig.getBucketName(), originalBlob.getName()), opts);
5760
FileChannel wc =
5861
FileChannel.open(
59-
path,
62+
destPath,
6063
StandardOpenOption.WRITE,
6164
StandardOpenOption.CREATE,
6265
StandardOpenOption.TRUNCATE_EXISTING)) {
@@ -89,7 +92,7 @@ public DownloadResult call() {
8992
}
9093
DownloadResult result =
9194
DownloadResult.newBuilder(originalBlob, TransferStatus.SUCCESS)
92-
.setOutputDestination(path.toAbsolutePath())
95+
.setOutputDestination(destPath)
9396
.build();
9497
return result;
9598
}

‎google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/ParallelDownloadConfig.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ public Builder setStripPrefix(String stripPrefix) {
159159
* @see ParallelDownloadConfig#getDownloadDirectory()
160160
*/
161161
public Builder setDownloadDirectory(Path downloadDirectory) {
162-
this.downloadDirectory = downloadDirectory;
162+
this.downloadDirectory = downloadDirectory.toAbsolutePath().normalize();
163163
return this;
164164
}
165165

‎google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/PathTraversalBlockedException.java‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
* Copyright 2026 Google LLC
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.cloud.storage.transfermanager;
18+
19+
import java.nio.file.Path;
20+
import java.util.Locale;
21+
22+
/**
23+
* Exception thrown when a download is blocked because the object name would result in a path
24+
* traversal outside the target directory.
25+
*/
26+
public final class PathTraversalBlockedException extends RuntimeException {
27+
28+
public PathTraversalBlockedException(String objectName, Path targetDirectory) {
29+
super(
30+
String.format(
31+
Locale.US,
32+
"Download of object '%s' was blocked because it would escape the target directory '%s'.",
33+
objectName,
34+
targetDirectory));
35+
}
36+
}

‎google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/TransferManagerImpl.java‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -145,15 +145,20 @@ public void close() throws Exception {
145145
Storage.BlobSourceOption[] opts =
146146
config.getOptionsPerRequest().toArray(new Storage.BlobSourceOption[0]);
147147
List<ApiFuture<DownloadResult>> downloadTasks = new ArrayList<>();
148-
if (!transferManagerConfig.isAllowDivideAndConquerDownload()) {
149-
for (BlobInfo blob : blobs) {
150-
DirectDownloadCallable callable = new DirectDownloadCallable(storage, blob, config, opts);
151-
downloadTasks.add(convert(executor.submit(callable)));
148+
for (BlobInfo blob : blobs) {
149+
Path destPath = TransferManagerUtils.createAndValidateDestPath(config, blob);
150+
if (destPath == null) {
151+
DownloadResult skipped =
152+
DownloadResult.newBuilder(blob, TransferStatus.FAILED_TO_START)
153+
.setException(
154+
new PathTraversalBlockedException(
155+
blob.getName(), config.getDownloadDirectory()))
156+
.build();
157+
downloadTasks.add(ApiFutures.immediateFuture(skipped));
158+
continue;
152159
}
153-
} else {
154-
for (BlobInfo blob : blobs) {
160+
if (transferManagerConfig.isAllowDivideAndConquerDownload()) {
155161
BlobInfo validatedBlob = retrieveSizeAndGeneration(storage, blob, config.getBucketName());
156-
Path destPath = TransferManagerUtils.createDestPath(config, blob);
157162
if (validatedBlob != null && qos.divideAndConquer(validatedBlob.getSize())) {
158163
DownloadResult optimisticResult =
159164
DownloadResult.newBuilder(validatedBlob, TransferStatus.SUCCESS)
@@ -181,12 +186,14 @@ public void close() throws Exception {
181186
DownloadSegment::reduce,
182187
BinaryOperator.minBy(DownloadResult.COMPARATOR)),
183188
MoreExecutors.directExecutor()));
184-
} else {
185-
DirectDownloadCallable callable = new DirectDownloadCallable(storage, blob, config, opts);
186-
downloadTasks.add(convert(executor.submit(callable)));
189+
continue;
187190
}
188191
}
192+
DirectDownloadCallable callable =
193+
new DirectDownloadCallable(storage, blob, config, opts, destPath);
194+
downloadTasks.add(convert(executor.submit(callable)));
189195
}
196+
190197
return DownloadJob.newBuilder()
191198
.setDownloadResults(downloadTasks)
192199
.setParallelDownloadConfig(config)

‎google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/TransferManagerUtils.java‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,18 @@ final class TransferManagerUtils {
2626

2727
private TransferManagerUtils() {}
2828

29-
static Path createDestPath(ParallelDownloadConfig config, BlobInfo originalBlob) {
29+
static Path createAndValidateDestPath(ParallelDownloadConfig config, BlobInfo originalBlob) {
30+
Path targetDirectory = config.getDownloadDirectory();
3031
Path newPath =
31-
config
32-
.getDownloadDirectory()
33-
.resolve(originalBlob.getName().replaceFirst(config.getStripPrefix(), ""));
32+
targetDirectory
33+
.resolve(originalBlob.getName().replaceFirst(config.getStripPrefix(), ""))
34+
.normalize();
35+
36+
// Security check: Verify the resolved path is inside the target directory
37+
// This catches ".." sequences that attempt to "escape" the folder.
38+
if (!newPath.startsWith(targetDirectory)) {
39+
return null;
40+
}
3441
// Check to make sure the parent directories exist
3542
if (Files.exists(newPath.getParent())) {
3643
return newPath;

‎google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITTransferManagerTest.java‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import com.google.cloud.storage.transfermanager.DownloadResult;
4242
import com.google.cloud.storage.transfermanager.ParallelDownloadConfig;
4343
import com.google.cloud.storage.transfermanager.ParallelUploadConfig;
44+
import com.google.cloud.storage.transfermanager.PathTraversalBlockedException;
4445
import com.google.cloud.storage.transfermanager.TransferManager;
4546
import com.google.cloud.storage.transfermanager.TransferManagerConfig;
4647
import com.google.cloud.storage.transfermanager.TransferManagerConfigTestingInstances;
@@ -635,6 +636,91 @@ public void bucketNameFromUploadBlobInfoFactoryMustMatchConfig() throws Exceptio
635636
}
636637
}
637638

639+
@Test
640+
public void downloadBlobsPathTraversalBlocked() throws Exception {
641+
TransferManagerConfig config =
642+
TransferManagerConfigTestingInstances.defaults(storage.getOptions());
643+
try (TransferManager transferManager = config.getService()) {
644+
String bucketName = bucket.getName();
645+
// Create an object with a name that attempts to "escape" the target directory
646+
String maliciousName = "../malicious.txt";
647+
BlobInfo maliciousBlob = BlobInfo.newBuilder(BlobId.of(bucketName, maliciousName)).build();
648+
storage.create(
649+
maliciousBlob, "malicious content".getBytes(java.nio.charset.StandardCharsets.UTF_8));
650+
651+
ParallelDownloadConfig parallelDownloadConfig =
652+
ParallelDownloadConfig.newBuilder()
653+
.setBucketName(bucketName)
654+
.setDownloadDirectory(baseDir) // baseDir is the target
655+
.build();
656+
657+
List<BlobInfo> blobsToDownload = new ArrayList<>(blobs);
658+
blobsToDownload.add(maliciousBlob);
659+
660+
DownloadJob job = transferManager.downloadBlobs(blobsToDownload, parallelDownloadConfig);
661+
List<DownloadResult> results = job.getDownloadResults();
662+
663+
try {
664+
long successCount =
665+
results.stream().filter(res -> res.getStatus() == TransferStatus.SUCCESS).count();
666+
assertThat(successCount).isEqualTo(blobs.size());
667+
668+
// Verify that the malicious blob was blocked/skipped
669+
Optional<DownloadResult> blockedResult =
670+
results.stream()
671+
.filter(res -> res.getInput().getName().equals(maliciousName))
672+
.findFirst();
673+
674+
assertThat(blockedResult).isPresent();
675+
assertThat(blockedResult.get().getStatus()).isEqualTo(TransferStatus.FAILED_TO_START);
676+
assertThat(blockedResult.get().getException())
677+
.isInstanceOf(PathTraversalBlockedException.class);
678+
assertThat(blockedResult.get().getException().getMessage()).contains("blocked");
679+
} finally {
680+
storage.delete(maliciousBlob.getBlobId());
681+
cleanUpFiles(
682+
results.stream()
683+
.filter(res -> res.getStatus() == TransferStatus.SUCCESS)
684+
.collect(Collectors.toList()));
685+
}
686+
}
687+
}
688+
689+
@Test
690+
public void downloadBlobsPathTraversalAllowedWithinTarget() throws Exception {
691+
TransferManagerConfig config =
692+
TransferManagerConfigTestingInstances.defaults(storage.getOptions());
693+
try (TransferManager transferManager = config.getService()) {
694+
String bucketName = bucket.getName();
695+
// This name resolves to 'safe.txt' inside the target directory
696+
String safeNameWithDots = "subdir/../safe.txt";
697+
BlobInfo safeBlob = BlobInfo.newBuilder(BlobId.of(bucketName, safeNameWithDots)).build();
698+
storage.create(safeBlob, "safe content".getBytes(java.nio.charset.StandardCharsets.UTF_8));
699+
700+
ParallelDownloadConfig parallelDownloadConfig =
701+
ParallelDownloadConfig.newBuilder()
702+
.setBucketName(bucketName)
703+
.setDownloadDirectory(baseDir)
704+
.build();
705+
706+
DownloadJob job =
707+
transferManager.downloadBlobs(
708+
Collections.singletonList(safeBlob), parallelDownloadConfig);
709+
List<DownloadResult> results = job.getDownloadResults();
710+
711+
try {
712+
assertThat(results.get(0).getStatus()).isEqualTo(TransferStatus.SUCCESS);
713+
// Verify it was saved to the correct normalized location
714+
Path expectedPath = baseDir.resolve("safe.txt").toAbsolutePath().normalize();
715+
assertThat(results.get(0).getOutputDestination().toAbsolutePath().normalize())
716+
.isEqualTo(expectedPath);
717+
} finally {
718+
cleanUpFiles(results);
719+
storage.delete(safeBlob.getBlobId());
720+
}
721+
}
722+
}
723+
638724
private void cleanUpFiles(List<DownloadResult> results) throws IOException {
639725
// Cleanup downloaded blobs and the parent directory
640726
for (DownloadResult res : results) {

0 commit comments

Comments
 (0)