This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch backport/CAMEL-24548-CAMEL-24549-4.18.x in repository https://gitbox.apache.org/repos/asf/camel.git
commit bfa6fece839faaa42a4c0ba3e15b1a1744f61069 Author: Claus Ibsen <[email protected]> AuthorDate: Tue Sep 1 07:38:03 2026 +0200 CAMEL-24548 CAMEL-24549: Harden cloud storage download containment Backport of #25873 to camel-4.18.x. 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, rejecting a symbolic link inside the download directory that resolves outside of it. This branch predates the shared camel-azure-common module and AzureFileNameHelper, so the Azure fix is applied directly to the duplicated resolveWithinDirectory() methods in BlobOperations and DataLakeFileOperations instead. The Google fix is a direct port, since GoogleCloudStorageFileNameHelper is unchanged from main. The upgrade-guide note was added to the existing "Upgrading from 4.18.4 to 4.18.5" section of camel-4x-upgrade-guide-4_18.adoc. Co-authored-by: Codex <[email protected]> Co-authored-by: Claude <[email protected]> Signed-off-by: Claus Ibsen <[email protected]> --- .../storage/blob/operations/BlobOperations.java | 39 +++++++++++++++++++- .../blob/operations/BlobOperationsTest.java | 21 +++++++++++ .../operations/DataLakeFileOperations.java | 39 +++++++++++++++++++- .../operations/DataLakeFileOperationTest.java | 21 +++++++++++ .../storage/GoogleCloudStorageFileNameHelper.java | 43 ++++++++++++++++++++-- .../GoogleCloudStorageFileNameHelperTest.java | 31 ++++++++++++++++ .../ROOT/pages/camel-4x-upgrade-guide-4_18.adoc | 13 +++++++ 7 files changed, 200 insertions(+), 7 deletions(-) diff --git a/components/camel-azure/camel-azure-storage-blob/src/main/java/org/apache/camel/component/azure/storage/blob/operations/BlobOperations.java b/components/camel-azure/camel-azure-storage-blob/src/main/java/org/apache/camel/component/azure/storage/blob/operations/BlobOperations.java index 59ede92f47fa..17bb0435d368 100644 --- a/components/camel-azure/camel-azure-storage-blob/src/main/java/org/apache/camel/component/azure/storage/blob/operations/BlobOperations.java +++ b/components/camel-azure/camel-azure-storage-blob/src/main/java/org/apache/camel/component/azure/storage/blob/operations/BlobOperations.java @@ -20,6 +20,8 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.nio.file.Files; +import java.nio.file.LinkOption; import java.nio.file.Path; import java.time.Duration; import java.time.OffsetDateTime; @@ -644,10 +646,43 @@ public class BlobOperations { 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-storage-blob/src/test/java/org/apache/camel/component/azure/storage/blob/operations/BlobOperationsTest.java b/components/camel-azure/camel-azure-storage-blob/src/test/java/org/apache/camel/component/azure/storage/blob/operations/BlobOperationsTest.java index 6c4b23671a29..f8744d961d0f 100644 --- a/components/camel-azure/camel-azure-storage-blob/src/test/java/org/apache/camel/component/azure/storage/blob/operations/BlobOperationsTest.java +++ b/components/camel-azure/camel-azure-storage-blob/src/test/java/org/apache/camel/component/azure/storage/blob/operations/BlobOperationsTest.java @@ -23,6 +23,8 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.Charset; +import java.nio.file.Files; +import java.nio.file.Path; import java.time.OffsetDateTime; import java.util.HashMap; import java.util.Map; @@ -45,6 +47,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; @@ -137,6 +140,24 @@ class BlobOperationsTest extends CamelTestSupport { assertThrows(IllegalArgumentException.class, () -> operations.downloadBlobToFile(exchange)); } + @Test + void testDownloadBlobToFileRejectsSymbolicLinkResolvingOutsideDirectory(@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); + + configuration.setFileDir(downloadDir.toString()); + when(client.getBlobName()).thenReturn("linked/PROOF_PWNED"); + + final BlobOperations operations = new BlobOperations(configuration, client); + final Exchange exchange = new DefaultExchange(context); + + final IllegalArgumentException exception + = assertThrows(IllegalArgumentException.class, () -> operations.downloadBlobToFile(exchange)); + assertTrue(exception.getMessage().contains("linked/PROOF_PWNED")); + assertTrue(exception.getMessage().contains(downloadDir.toString())); + } + @Test void testUploadBlockBlob() throws Exception { // mocking diff --git a/components/camel-azure/camel-azure-storage-datalake/src/main/java/org/apache/camel/component/azure/storage/datalake/operations/DataLakeFileOperations.java b/components/camel-azure/camel-azure-storage-datalake/src/main/java/org/apache/camel/component/azure/storage/datalake/operations/DataLakeFileOperations.java index 6a2c92a403af..58749f17d44e 100644 --- a/components/camel-azure/camel-azure-storage-datalake/src/main/java/org/apache/camel/component/azure/storage/datalake/operations/DataLakeFileOperations.java +++ b/components/camel-azure/camel-azure-storage-datalake/src/main/java/org/apache/camel/component/azure/storage/datalake/operations/DataLakeFileOperations.java @@ -20,6 +20,8 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.nio.file.Files; +import java.nio.file.LinkOption; import java.nio.file.OpenOption; import java.nio.file.Path; import java.time.Duration; @@ -262,10 +264,43 @@ public class DataLakeFileOperations { 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-storage-datalake/src/test/java/org/apache/camel/component/azure/storage/datalake/operations/DataLakeFileOperationTest.java b/components/camel-azure/camel-azure-storage-datalake/src/test/java/org/apache/camel/component/azure/storage/datalake/operations/DataLakeFileOperationTest.java index d2db2a7ac0f8..3b8c131548bc 100644 --- a/components/camel-azure/camel-azure-storage-datalake/src/test/java/org/apache/camel/component/azure/storage/datalake/operations/DataLakeFileOperationTest.java +++ b/components/camel-azure/camel-azure-storage-datalake/src/test/java/org/apache/camel/component/azure/storage/datalake/operations/DataLakeFileOperationTest.java @@ -22,6 +22,8 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.Charset; +import java.nio.file.Files; +import java.nio.file.Path; import java.time.OffsetDateTime; import com.azure.core.http.HttpHeaders; @@ -39,6 +41,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; @@ -107,6 +110,24 @@ public class DataLakeFileOperationTest extends CamelTestSupport { assertThrows(IllegalArgumentException.class, () -> operations.downloadToFile(exchange)); } + @Test + void testDownloadToFileRejectsSymbolicLinkResolvingOutsideDirectory(@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); + + configuration.setFileDir(downloadDir.toString()); + when(client.getFileName()).thenReturn("linked/PROOF_PWNED"); + + final DataLakeFileOperations operations = new DataLakeFileOperations(configuration, client); + final Exchange exchange = new DefaultExchange(context); + + final IllegalArgumentException exception + = assertThrows(IllegalArgumentException.class, () -> operations.downloadToFile(exchange)); + assertTrue(exception.getMessage().contains("linked/PROOF_PWNED")); + assertTrue(exception.getMessage().contains(downloadDir.toString())); + } + @Test void testUploadFile() throws Exception { final OffsetDateTime time = OffsetDateTime.now(); 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_18.adoc b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_18.adoc index ea02914342af..bf669caf9230 100644 --- a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_18.adoc +++ b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_18.adoc @@ -33,6 +33,19 @@ The same default is now also applied to the `ClientConfig` that Camel builds for endpoints, when neither a referenced `ClientConfig` nor `hazelcastConfigUri` is supplied. Client mode previously behaved differently from node mode for an otherwise identical endpoint configuration. +=== 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 from 4.18.3 to 4.18.4 === camel-core - Multicast EIP honors UseOriginalAggregationStrategy
