This is an automated email from the ASF dual-hosted git repository.
Croway pushed a commit to branch camel-4.18.x
in repository https://gitbox.apache.org/repos/asf/camel.git
The following commit(s) were added to refs/heads/camel-4.18.x by this push:
new 3c322a1419c9 CAMEL-24548 CAMEL-24549: Harden cloud storage download
containment
3c322a1419c9 is described below
commit 3c322a1419c9037c4b3aa479d180114db0b2a7f9
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
