This is an automated email from the ASF dual-hosted git repository.
davsclaus pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel.git
The following commit(s) were added to refs/heads/main by this push:
new 299fd780945c CAMEL-24548 CAMEL-24549: Harden cloud storage download
containment
299fd780945c is described below
commit 299fd780945c0bc2b0f38eced56e16a91f0d5f86
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_23.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_23.adoc
b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
index 525aa3b8a8ea..c193671874f8 100644
--- a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
+++ b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
@@ -1097,3 +1097,16 @@ file. Converting a `String` to `Reader` itself, or to a
non-file-backed subclass
unaffected.
`String` to `java.io.File` is unchanged: there the `String` genuinely is a
path.
+
+=== 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.
