This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch backport/CAMEL-24548-CAMEL-24549-4.22.x in repository https://gitbox.apache.org/repos/asf/camel.git
commit 5970150cea52a7205a0d802628d6390f5abe2a57 Author: Andrea Cosentino <[email protected]> AuthorDate: Tue Sep 1 07:27:35 2026 +0200 CAMEL-24548 CAMEL-24549: Harden cloud storage download containment Local downloads in camel-azure-storage-blob, camel-azure-storage-datalake, and camel-google-storage now resolve existing filesystem path segments before checking that the destination stays inside the configured directory. Previously only the raw, unresolved destination path was checked, so a symbolic link inside the download directory that resolved outside of it could be used to write files elsewhere on the filesystem. Valid nested download paths and object names using "/" as a pseudo-directory separator continue to work unchanged. Co-authored-by: Codex <[email protected]> Closes #25873 --- .../azure/common/AzureFileNameHelper.java | 40 +++++++++++++++++++- .../azure/common/AzureFileNameHelperTest.java | 26 +++++++++++++ .../storage/GoogleCloudStorageFileNameHelper.java | 43 ++++++++++++++++++++-- .../GoogleCloudStorageFileNameHelperTest.java | 31 ++++++++++++++++ .../ROOT/pages/camel-4x-upgrade-guide-4_22.adoc | 13 +++++++ 5 files changed, 148 insertions(+), 5 deletions(-) diff --git a/components/camel-azure/camel-azure-common/src/main/java/org/apache/camel/component/azure/common/AzureFileNameHelper.java b/components/camel-azure/camel-azure-common/src/main/java/org/apache/camel/component/azure/common/AzureFileNameHelper.java index 371f7f9faa56..ca5aa2508abe 100644 --- a/components/camel-azure/camel-azure-common/src/main/java/org/apache/camel/component/azure/common/AzureFileNameHelper.java +++ b/components/camel-azure/camel-azure-common/src/main/java/org/apache/camel/component/azure/common/AzureFileNameHelper.java @@ -17,6 +17,9 @@ package org.apache.camel.component.azure.common; import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.LinkOption; import java.nio.file.Path; /** @@ -44,10 +47,43 @@ public final class AzureFileNameHelper { final Path normalizedDir = new File(fileDir).toPath().normalize(); final Path normalizedTarget = target.toPath().normalize(); if (!normalizedTarget.startsWith(normalizedDir)) { + throw outsideDirectory(name, fileDir); + } + + try { + final Path resolvedDir = resolveExistingPathSegments(new File(fileDir).toPath()); + final Path resolvedTarget = resolveExistingPathSegments(target.toPath()); + if (!resolvedTarget.startsWith(resolvedDir)) { + throw outsideDirectory(name, fileDir); + } + } catch (IOException e) { throw new IllegalArgumentException( - "Cannot download to file '" + name - + "' as it resolves outside the configured fileDir directory: " + fileDir); + "Cannot verify download path for file '" + name + "' within the configured fileDir directory: " + + fileDir, + e); } return target; } + + private static Path resolveExistingPathSegments(Path path) throws IOException { + // Preserve the raw path segments here. Normalizing before resolving links changes the filesystem meaning of + // paths such as link/../file when link points to another directory. + final Path absolutePath = path.toAbsolutePath(); + Path existingPath = absolutePath; + while (existingPath != null && !Files.exists(existingPath, LinkOption.NOFOLLOW_LINKS)) { + existingPath = existingPath.getParent(); + } + if (existingPath == null) { + throw new IOException("No existing ancestor found for " + path); + } + + final Path resolvedExistingPath = existingPath.toRealPath(); + return resolvedExistingPath.resolve(existingPath.relativize(absolutePath)).normalize(); + } + + private static IllegalArgumentException outsideDirectory(String name, String fileDir) { + return new IllegalArgumentException( + "Cannot download to file '" + name + + "' as it resolves outside the configured fileDir directory: " + fileDir); + } } diff --git a/components/camel-azure/camel-azure-common/src/test/java/org/apache/camel/component/azure/common/AzureFileNameHelperTest.java b/components/camel-azure/camel-azure-common/src/test/java/org/apache/camel/component/azure/common/AzureFileNameHelperTest.java index 4668800d6e70..53c12bc9c6b1 100644 --- a/components/camel-azure/camel-azure-common/src/test/java/org/apache/camel/component/azure/common/AzureFileNameHelperTest.java +++ b/components/camel-azure/camel-azure-common/src/test/java/org/apache/camel/component/azure/common/AzureFileNameHelperTest.java @@ -17,6 +17,8 @@ package org.apache.camel.component.azure.common; import java.io.File; +import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; import org.junit.jupiter.api.Test; @@ -72,4 +74,28 @@ class AzureFileNameHelperTest { assertThrows(IllegalArgumentException.class, () -> AzureFileNameHelper.resolveWithinDirectory(fileDir, "../workspace/secret")); } + + @Test + void shouldRejectSymbolicLinkResolvingOutsideDirectory(@TempDir Path parent) throws IOException { + Path downloadDir = Files.createDirectory(parent.resolve("downloads")); + Path outsideDir = Files.createDirectory(parent.resolve("outside")); + Files.createSymbolicLink(downloadDir.resolve("linked"), outsideDir); + + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> AzureFileNameHelper.resolveWithinDirectory(downloadDir.toString(), "linked/file.txt")); + assertTrue(exception.getMessage().contains("linked/file.txt")); + assertTrue(exception.getMessage().contains(downloadDir.toString())); + } + + @Test + void shouldResolveParentSegmentAfterSymbolicLink(@TempDir Path parent) throws IOException { + Path downloadDir = Files.createDirectory(parent.resolve("downloads")); + Path outsideDir = Files.createDirectories(parent.resolve("outside/child")); + Files.createSymbolicLink(downloadDir.resolve("linked"), outsideDir); + + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> AzureFileNameHelper.resolveWithinDirectory(downloadDir.toString(), "linked/../file.txt")); + assertTrue(exception.getMessage().contains("linked/../file.txt")); + assertTrue(exception.getMessage().contains(downloadDir.toString())); + } } diff --git a/components/camel-google/camel-google-storage/src/main/java/org/apache/camel/component/google/storage/GoogleCloudStorageFileNameHelper.java b/components/camel-google/camel-google-storage/src/main/java/org/apache/camel/component/google/storage/GoogleCloudStorageFileNameHelper.java index 59ca3a2b6e2e..53165cefa813 100644 --- a/components/camel-google/camel-google-storage/src/main/java/org/apache/camel/component/google/storage/GoogleCloudStorageFileNameHelper.java +++ b/components/camel-google/camel-google-storage/src/main/java/org/apache/camel/component/google/storage/GoogleCloudStorageFileNameHelper.java @@ -17,6 +17,9 @@ package org.apache.camel.component.google.storage; import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.LinkOption; import java.nio.file.Path; /** @@ -47,10 +50,44 @@ final class GoogleCloudStorageFileNameHelper { final Path normalizedDir = new File(downloadDirectory).toPath().normalize(); final Path normalizedTarget = new File(resolvedPath).toPath().normalize(); if (!normalizedTarget.startsWith(normalizedDir)) { + throw outsideDirectory(objectName, downloadDirectory); + } + + try { + final Path resolvedDir = resolveExistingPathSegments(new File(downloadDirectory).toPath()); + final Path resolvedTarget = resolveExistingPathSegments(new File(resolvedPath).toPath()); + if (!resolvedTarget.startsWith(resolvedDir)) { + throw outsideDirectory(objectName, downloadDirectory); + } + } catch (IOException e) { throw new IllegalArgumentException( - "Cannot download to file '" + objectName - + "' as it resolves outside the configured downloadFileName directory: " - + downloadDirectory); + "Cannot verify download path for file '" + objectName + + "' within the configured downloadFileName directory: " + + downloadDirectory, + e); + } + } + + private static Path resolveExistingPathSegments(Path path) throws IOException { + // Preserve the raw path segments here. Normalizing before resolving links changes the filesystem meaning of + // paths such as link/../file when link points to another directory. + final Path absolutePath = path.toAbsolutePath(); + Path existingPath = absolutePath; + while (existingPath != null && !Files.exists(existingPath, LinkOption.NOFOLLOW_LINKS)) { + existingPath = existingPath.getParent(); } + if (existingPath == null) { + throw new IOException("No existing ancestor found for " + path); + } + + final Path resolvedExistingPath = existingPath.toRealPath(); + return resolvedExistingPath.resolve(existingPath.relativize(absolutePath)).normalize(); + } + + private static IllegalArgumentException outsideDirectory(String objectName, String downloadDirectory) { + return new IllegalArgumentException( + "Cannot download to file '" + objectName + + "' as it resolves outside the configured downloadFileName directory: " + + downloadDirectory); } } diff --git a/components/camel-google/camel-google-storage/src/test/java/org/apache/camel/component/google/storage/GoogleCloudStorageFileNameHelperTest.java b/components/camel-google/camel-google-storage/src/test/java/org/apache/camel/component/google/storage/GoogleCloudStorageFileNameHelperTest.java index d84c9e42468d..167f9263e9df 100644 --- a/components/camel-google/camel-google-storage/src/test/java/org/apache/camel/component/google/storage/GoogleCloudStorageFileNameHelperTest.java +++ b/components/camel-google/camel-google-storage/src/test/java/org/apache/camel/component/google/storage/GoogleCloudStorageFileNameHelperTest.java @@ -16,7 +16,12 @@ */ package org.apache.camel.component.google.storage; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -80,4 +85,30 @@ class GoogleCloudStorageFileNameHelperTest { .isThrownBy(() -> GoogleCloudStorageFileNameHelper.assertWithinDirectory(DIR, DIR + "-evil/file.txt", "../gcs-download-evil/file.txt")); } + + @Test + void symbolicLinkResolvingOutsideDirectoryIsRejected(@TempDir Path parent) throws IOException { + Path downloadDir = Files.createDirectory(parent.resolve("downloads")); + Path outsideDir = Files.createDirectory(parent.resolve("outside")); + Path linkedPath = Files.createSymbolicLink(downloadDir.resolve("linked"), outsideDir); + + assertThatIllegalArgumentException() + .isThrownBy(() -> GoogleCloudStorageFileNameHelper.assertWithinDirectory( + downloadDir.toString(), linkedPath.resolve("file.txt").toString(), "linked/file.txt")) + .withMessageContaining("linked/file.txt") + .withMessageContaining(downloadDir.toString()); + } + + @Test + void parentSegmentAfterSymbolicLinkIsResolvedByFilesystem(@TempDir Path parent) throws IOException { + Path downloadDir = Files.createDirectory(parent.resolve("downloads")); + Path outsideDir = Files.createDirectories(parent.resolve("outside/child")); + Path linkedPath = Files.createSymbolicLink(downloadDir.resolve("linked"), outsideDir); + + assertThatIllegalArgumentException() + .isThrownBy(() -> GoogleCloudStorageFileNameHelper.assertWithinDirectory( + downloadDir.toString(), linkedPath.resolve("../file.txt").toString(), "linked/../file.txt")) + .withMessageContaining("linked/../file.txt") + .withMessageContaining(downloadDir.toString()); + } } diff --git a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_22.adoc b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_22.adoc index 625ae75e2620..c98771e8780f 100644 --- a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_22.adoc +++ b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_22.adoc @@ -45,6 +45,19 @@ When the flag is `false` (the default), any remaining `CamelExecCommand*`, and a WARN is logged once per exec endpoint. Those headers never overrode the URI without the flag; they were just silent before. +=== camel-azure-storage-blob and camel-azure-storage-datalake + +Local downloads configured with `fileDir` now resolve existing filesystem path segments before checking +that the destination remains inside the configured directory. Downloads through a symbolic link that +resolves outside `fileDir` are rejected. Valid nested download paths continue to work. + +=== camel-google-storage + +Local downloads configured with a plain `downloadFileName` directory now resolve existing filesystem +path segments before checking that the destination remains inside that directory. Downloads through a +symbolic link that resolves outside the configured directory are rejected. Valid object names using `/` +as a pseudo-directory separator continue to work. + == Upgrading Camel 4.21 to 4.22 === camel-tika
