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 b049bc9e49f0 CAMEL-25163: camel-google-storage - apply the download
directory containment check on the expression branch (#27119)
b049bc9e49f0 is described below
commit b049bc9e49f002770761067bbac8dddedbd5caa3
Author: Andrea Cosentino <[email protected]>
AuthorDate: Fri Oct 2 14:23:08 2026 +0200
CAMEL-25163: camel-google-storage - apply the download directory
containment check on the expression branch (#27119)
Co-Authored-By: Claude Opus 5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
---
.../google/storage/GoogleCloudStorageConsumer.java | 57 ++++--
.../storage/GoogleCloudStorageFileNameHelper.java | 81 +++++++++
...GoogleCloudStorageConsumerDownloadPathTest.java | 195 ++++++++++++++++++++-
.../GoogleCloudStorageFileNameHelperTest.java | 94 ++++++++++
.../ROOT/pages/camel-4x-upgrade-guide-4_23.adoc | 16 ++
5 files changed, 427 insertions(+), 16 deletions(-)
diff --git
a/components/camel-google/camel-google-storage/src/main/java/org/apache/camel/component/google/storage/GoogleCloudStorageConsumer.java
b/components/camel-google/camel-google-storage/src/main/java/org/apache/camel/component/google/storage/GoogleCloudStorageConsumer.java
index c6bd1b1f8c6d..8a5ed9f40708 100644
---
a/components/camel-google/camel-google-storage/src/main/java/org/apache/camel/component/google/storage/GoogleCloudStorageConsumer.java
+++
b/components/camel-google/camel-google-storage/src/main/java/org/apache/camel/component/google/storage/GoogleCloudStorageConsumer.java
@@ -154,8 +154,17 @@ public class GoogleCloudStorageConsumer extends
ScheduledBatchPollingConsumer {
try {
for (Blob blob : blobList) {
if (includeObject(blob)) {
- Exchange exchange = createExchange(blob,
blob.getBlobId().getName());
- answer.add(exchange);
+ String key = blob.getBlobId().getName();
+ try {
+ answer.add(createExchange(blob, key));
+ } catch (IllegalArgumentException e) {
+ // the object name is rejected as a local download
path: skip this object only, so a single
+ // object with such a name does not stop the rest of
the bucket from being consumed
+ getExceptionHandler().handleException(
+ "Skipping object " + key + " in bucket " +
getConfiguration().getBucketName()
+ + " as it cannot
be downloaded: " + e.getMessage(),
+ e);
+ }
}
}
} catch (Exception e) {
@@ -306,7 +315,14 @@ public class GoogleCloudStorageConsumer extends
ScheduledBatchPollingConsumer {
// download as file
if (getConfiguration().getDownloadFileName() != null) {
// create a dummy exchange as Exchange is needed for
expression evaluation
- String result = evaluateFileExpression(exchange,
getConfiguration().getDownloadFileName(), blob.getName());
+ String result;
+ try {
+ result = evaluateFileExpression(exchange,
getConfiguration().getDownloadFileName(), blob.getName());
+ } catch (IllegalArgumentException e) {
+ // the object is not consumed, so the exchange created for
it is not routed either
+ releaseExchange(exchange, false);
+ throw e;
+ }
if (result != null) {
File file = new File(result);
blob.downloadTo(file.toPath());
@@ -360,13 +376,30 @@ public class GoogleCloudStorageConsumer extends
ScheduledBatchPollingConsumer {
// use blob as file name
exchange.getMessage().setHeader(GoogleCloudStorageConstants.FILE_NAME,
blogName);
+ // the local path is resolved from
GoogleCloudStorageConstants.FILE_NAME set above, which carries the remote
+ // object name and is therefore untrusted input, no matter whether the
token is appended here or already part
+ // of the configured downloadFileName. An absolute object name, or one
with a .. segment, is rejected whatever
+ // the configuration looks like, and the resolved path is then
confined:
+ // - plain directory (no expression): the object name is appended to
it and the configured value itself is the
+ // directory the download must stay within
+ // - directory followed by an expression (for example
/tmp/downloads/${file:name}): the static directory
+ // prefix before the first expression token is the directory the
download must stay within
+ // - fully dynamic value with no static directory prefix (for example
${file:name}): the route author did not
+ // configure any directory, so a relative result must stay within
the working directory
+ if (blogName != null) {
+ GoogleCloudStorageFileNameHelper.assertSafeObjectName(blogName);
+ }
+
String eval = downloadFileName;
- // when the configured downloadFileName is a plain directory, the
remote object name is appended to it. That
- // name is untrusted input, so the resolved path has to be confined to
the configured directory. When the
- // configuration already contains an expression the local path is
built by the route author, who is trusted.
- boolean confineToDirectory = !downloadFileName.contains("$");
- if (confineToDirectory) {
+ final String confinementDirectory;
+ final boolean confineToDirectory;
+ if (downloadFileName.contains("$")) {
+ confinementDirectory =
GoogleCloudStorageFileNameHelper.staticDirectoryPrefix(downloadFileName);
+ confineToDirectory = !confinementDirectory.isEmpty();
+ } else {
eval = downloadFileName + "/${file:name}";
+ confinementDirectory = downloadFileName;
+ confineToDirectory = true;
}
Expression exp = language.createExpression(eval);
exp.init(camelContext);
@@ -375,8 +408,12 @@ public class GoogleCloudStorageConsumer extends
ScheduledBatchPollingConsumer {
if (exchange.getException() != null) {
throw
RuntimeCamelException.wrapRuntimeCamelException(exchange.getException());
}
- if (confineToDirectory && result != null) {
-
GoogleCloudStorageFileNameHelper.assertWithinDirectory(downloadFileName,
result, blogName);
+ if (result != null) {
+ if (confineToDirectory) {
+
GoogleCloudStorageFileNameHelper.assertWithinDirectory(confinementDirectory,
result, blogName);
+ } else {
+
GoogleCloudStorageFileNameHelper.assertWithinWorkingDirectory(result, blogName);
+ }
}
return result;
}
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 53165cefa813..3a580dab3606 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
@@ -30,6 +30,55 @@ final class GoogleCloudStorageFileNameHelper {
private GoogleCloudStorageFileNameHelper() {
}
+ /**
+ * Rejects a remote object name that could steer a local download path out
of the directory it is resolved in,
+ * whatever the configured {@code downloadFileName} looks like. That is an
absolute object name (one starting with
+ * {@code /} or {@code \}, or with a drive letter such as {@code C:}) or
an object name with a {@code ..} path
+ * segment, even one that would normalize back inside the directory. Both
{@code /} and {@code \} are treated as
+ * separators so the check does not depend on the platform the consumer
runs on.
+ * <p>
+ * This is what confines a fully dynamic {@code downloadFileName} such as
{@code ${file:name}}, which has no
+ * configured directory to check the resolved path against.
+ *
+ * @param objectName the remote object name
+ * @throws IllegalArgumentException if the object name is absolute or has
a {@code ..} path segment
+ */
+ static void assertSafeObjectName(String objectName) {
+ if (objectName.startsWith("/") || objectName.startsWith("\\") ||
hasDriveLetter(objectName)) {
+ throw new IllegalArgumentException(
+ "Cannot download to file '" + objectName + "' as the
object name is an absolute path");
+ }
+ for (String segment : objectName.split("[/\\\\]")) {
+ if ("..".equals(segment)) {
+ throw new IllegalArgumentException(
+ "Cannot download to file '" + objectName + "' as the
object name has a '..' path segment");
+ }
+ }
+ }
+
+ /**
+ * Verifies that a relative local download path does not climb out of the
working directory. This applies to a fully
+ * dynamic {@code downloadFileName} such as {@code ${file:name}}, which
has no configured directory to confine the
+ * download to. {@link #assertSafeObjectName(String)} already keeps the
object name itself from climbing out; this
+ * catches an object name that joins with the configured text into a
parent segment, for example
+ * {@code .${file:name}} with an object named {@code ./file.txt}, which
resolves to {@code ../file.txt}.
+ * <p>
+ * The check is lexical on purpose: an absolute path comes from the route
author's own configuration, and symbolic
+ * links inside the working directory belong to the deployment, not to the
remote object name.
+ *
+ * @param resolvedPath the resolved local path
+ * @param objectName the remote object name used to build
the local path, for error reporting
+ * @throws IllegalArgumentException if the relative path resolves outside
the working directory
+ */
+ static void assertWithinWorkingDirectory(String resolvedPath, String
objectName) {
+ final Path normalized = new File(resolvedPath).toPath().normalize();
+ if (!normalized.isAbsolute() && normalized.startsWith("..")) {
+ throw new IllegalArgumentException(
+ "Cannot download to file '" + objectName + "' as it
resolves outside the working directory: "
+ + resolvedPath);
+ }
+ }
+
/**
* Verifies that a local download path built from a remote object name
stays within the configured download
* directory. A remote object name is influenced by whoever writes to the
bucket and may contain path segments that
@@ -68,6 +117,38 @@ final class GoogleCloudStorageFileNameHelper {
}
}
+ /**
+ * Extracts the static directory prefix of a configured {@code
downloadFileName} that contains an expression token,
+ * that is the part before the first {@code $} trimmed back to the last
path separator. Trimming back to a separator
+ * is required so that a partial path segment is not mistaken for a
directory: {@code /tmp/down${file:name}} has
+ * {@code /tmp} as its static directory prefix, not {@code /tmp/down}.
+ *
+ * @param downloadFileName the configured {@code downloadFileName}
containing at least one expression token
+ * @return the static directory prefix, or an empty
string when the configured value is fully
+ * dynamic and has no static directory prefix
(for example {@code ${file:name}})
+ */
+ static String staticDirectoryPrefix(String downloadFileName) {
+ final String beforeExpression = downloadFileName.substring(0,
downloadFileName.indexOf('$'));
+ final int lastSeparator = Math.max(beforeExpression.lastIndexOf('/'),
beforeExpression.lastIndexOf('\\'));
+ if (lastSeparator < 0) {
+ return "";
+ }
+ if (lastSeparator == 0) {
+ // the prefix is the filesystem root itself
+ return beforeExpression.substring(0, 1);
+ }
+ if (lastSeparator == 2 && hasDriveLetter(beforeExpression)) {
+ // the prefix is a Windows drive root such as C:\ - keep the
separator, as C: alone is drive-relative (the
+ // current directory on that drive) rather than the root
+ return beforeExpression.substring(0, 3);
+ }
+ return beforeExpression.substring(0, lastSeparator);
+ }
+
+ private static boolean hasDriveLetter(String path) {
+ return path.length() >= 2 && path.charAt(1) == ':' &&
Character.isLetter(path.charAt(0));
+ }
+
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.
diff --git
a/components/camel-google/camel-google-storage/src/test/java/org/apache/camel/component/google/storage/GoogleCloudStorageConsumerDownloadPathTest.java
b/components/camel-google/camel-google-storage/src/test/java/org/apache/camel/component/google/storage/GoogleCloudStorageConsumerDownloadPathTest.java
index 71a1f6a6d75a..399654bdb8e1 100644
---
a/components/camel-google/camel-google-storage/src/test/java/org/apache/camel/component/google/storage/GoogleCloudStorageConsumerDownloadPathTest.java
+++
b/components/camel-google/camel-google-storage/src/test/java/org/apache/camel/component/google/storage/GoogleCloudStorageConsumerDownloadPathTest.java
@@ -16,9 +16,21 @@
*/
package org.apache.camel.component.google.storage;
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Queue;
+
+import com.google.cloud.storage.Blob;
+import com.google.cloud.storage.BlobInfo;
+import com.google.cloud.storage.Storage;
import org.apache.camel.CamelContext;
import org.apache.camel.Exchange;
import
org.apache.camel.component.google.storage.localstorage.LocalStorageHelper;
+import org.apache.camel.spi.ExceptionHandler;
import org.apache.camel.support.DefaultExchange;
import org.apache.camel.test.junit6.CamelTestSupport;
import org.junit.jupiter.api.Test;
@@ -88,14 +100,185 @@ class GoogleCloudStorageConsumerDownloadPathTest extends
CamelTestSupport {
}
@Test
- void routeAuthorSuppliedExpressionIsNotConfined() throws Exception {
- // a downloadFileName that already contains an expression is built by
the route author, who is trusted, so it is
- // evaluated as configured and deliberately left outside the
containment check
- String expression = "target/${file:name}";
+ void plainObjectNameResolvesInsideStaticPrefixOfExpression() throws
Exception {
+ // the ${file:name} token resolves to the remote object name, so the
static directory prefix of the configured
+ // downloadFileName confines the download just like a plain directory
does
+ String expression = DOWNLOAD_DIR + "/${file:name}";
+ GoogleCloudStorageConsumer consumer = createConsumer(DOWNLOAD_DIR);
+ Exchange exchange = new DefaultExchange(context);
+
+ assertThat(consumer.evaluateFileExpression(exchange, expression,
"file.txt"))
+ .isEqualTo(DOWNLOAD_DIR + "/file.txt");
+ }
+
+ @Test
+ void nestedObjectNameResolvesInsideStaticPrefixOfExpression() throws
Exception {
+ String expression = DOWNLOAD_DIR + "/${file:name}";
+ GoogleCloudStorageConsumer consumer = createConsumer(DOWNLOAD_DIR);
+ Exchange exchange = new DefaultExchange(context);
+
+ assertThat(consumer.evaluateFileExpression(exchange, expression,
"nested/file.txt"))
+ .isEqualTo(DOWNLOAD_DIR + "/nested/file.txt");
+ }
+
+ @Test
+ void objectNameWithParentSegmentIsRejectedOnTheExpressionBranch() throws
Exception {
+ String expression = DOWNLOAD_DIR + "/${file:name}";
+ GoogleCloudStorageConsumer consumer = createConsumer(DOWNLOAD_DIR);
+ Exchange exchange = new DefaultExchange(context);
+
+ assertThatIllegalArgumentException()
+ .isThrownBy(() -> consumer.evaluateFileExpression(exchange,
expression, "../escape.txt"))
+ .withMessageContaining("../escape.txt");
+ }
+
+ @Test
+ void objectNameJoiningTheExpressionOutOfTheStaticPrefixIsRejected() throws
Exception {
+ // ./escape.txt has no .. segment of its own, but /.${file:name} turns
it into a parent segment, so the static
+ // directory prefix still has to confine the result
+ String expression = DOWNLOAD_DIR + "/.${file:name}";
+ GoogleCloudStorageConsumer consumer = createConsumer(DOWNLOAD_DIR);
+ Exchange exchange = new DefaultExchange(context);
+
+ assertThatIllegalArgumentException()
+ .isThrownBy(() -> consumer.evaluateFileExpression(exchange,
expression, "./escape.txt"))
+ .withMessageContaining("./escape.txt")
+ .withMessageContaining(DOWNLOAD_DIR);
+ }
+
+ @Test
+ void
objectNameWithParentSegmentNestedInTheKeyIsRejectedOnTheExpressionBranch()
throws Exception {
+ String expression = DOWNLOAD_DIR + "/${file:name}";
+ GoogleCloudStorageConsumer consumer = createConsumer(DOWNLOAD_DIR);
+ Exchange exchange = new DefaultExchange(context);
+
+ assertThatIllegalArgumentException()
+ .isThrownBy(() -> consumer.evaluateFileExpression(exchange,
expression, "nested/../../escape.txt"));
+ }
+
+ @Test
+ void objectNameNormalizingBackInsideIsRejected() throws Exception {
+ // a .. segment is refused outright, even when it would normalize back
inside the download directory
+ GoogleCloudStorageConsumer consumer = createConsumer(DOWNLOAD_DIR);
+ Exchange exchange = new DefaultExchange(context);
+
+ assertThatIllegalArgumentException()
+ .isThrownBy(() -> consumer.evaluateFileExpression(exchange,
DOWNLOAD_DIR, "nested/../file.txt"))
+ .withMessageContaining("nested/../file.txt");
+ }
+
+ @Test
+ void plainObjectNameOnAFullyDynamicExpressionIsAccepted() throws Exception
{
+ String expression = "${file:name}";
+ GoogleCloudStorageConsumer consumer = createConsumer(DOWNLOAD_DIR);
+ Exchange exchange = new DefaultExchange(context);
+
+ assertThat(consumer.evaluateFileExpression(exchange, expression,
"nested/file.txt"))
+ .isEqualTo("nested/file.txt");
+ }
+
+ @Test
+ void objectNameWithParentSegmentIsRejectedOnAFullyDynamicExpression()
throws Exception {
+ // the route author configured no directory, but the object name is
still untrusted input
+ String expression = "${file:name}";
+ GoogleCloudStorageConsumer consumer = createConsumer(DOWNLOAD_DIR);
+ Exchange exchange = new DefaultExchange(context);
+
+ assertThatIllegalArgumentException()
+ .isThrownBy(() -> consumer.evaluateFileExpression(exchange,
expression,
+ "../../home/app/.ssh/authorized_keys"))
+ .withMessageContaining("../../home/app/.ssh/authorized_keys");
+ }
+
+ @Test
+ void absoluteObjectNameIsRejectedOnAFullyDynamicExpression() throws
Exception {
+ String expression = "${file:name}";
+ GoogleCloudStorageConsumer consumer = createConsumer(DOWNLOAD_DIR);
+ Exchange exchange = new DefaultExchange(context);
+
+ assertThatIllegalArgumentException()
+ .isThrownBy(() -> consumer.evaluateFileExpression(exchange,
expression, "/etc/cron.d/escape"))
+ .withMessageContaining("/etc/cron.d/escape");
+ }
+
+ @Test
+ void absoluteObjectNameAfterAFileNamePrefixIsRejected() throws Exception {
+ // prefix-/../../escape.txt would normalize to ../escape.txt
+ String expression = "prefix-${file:name}";
+ GoogleCloudStorageConsumer consumer = createConsumer(DOWNLOAD_DIR);
+ Exchange exchange = new DefaultExchange(context);
+
+ assertThatIllegalArgumentException()
+ .isThrownBy(() -> consumer.evaluateFileExpression(exchange,
expression, "/../../escape.txt"));
+ }
+
+ @Test
+ void objectNameJoiningTheExpressionIntoAParentSegmentIsRejected() throws
Exception {
+ // ./file.txt has no .. segment of its own, but .${file:name} turns it
into ../file.txt
+ String expression = ".${file:name}";
+ GoogleCloudStorageConsumer consumer = createConsumer(DOWNLOAD_DIR);
+ Exchange exchange = new DefaultExchange(context);
+
+ assertThatIllegalArgumentException()
+ .isThrownBy(() -> consumer.evaluateFileExpression(exchange,
expression, "./file.txt"))
+ .withMessageContaining("working directory");
+ }
+
+ @Test
+ void objectWithARejectedNameIsSkippedWithoutStoppingTheOthers() throws
Exception {
+ // the rejected object is reported and skipped, while the other
objects of the same poll are still consumed
+ // (the accepted objects are really downloaded, and the consumer does
not create the download directory)
+ Files.createDirectories(Path.of(DOWNLOAD_DIR));
+ GoogleCloudStorageEndpoint endpoint = context.getEndpoint(
+ "google-storage://myRejectBucket?autoCreateBucket=true",
GoogleCloudStorageEndpoint.class);
+ endpoint.getConfiguration().setDownloadFileName(DOWNLOAD_DIR +
"/${exchangeId}.bin");
+ GoogleCloudStorageConsumer consumer = (GoogleCloudStorageConsumer)
endpoint.createConsumer(exchange -> {
+ });
+ endpoint.start();
+ consumer.init();
+ List<String> reported = new ArrayList<>();
+ consumer.setExceptionHandler(new ExceptionHandler() {
+ @Override
+ public void handleException(Throwable exception) {
+ reported.add(exception.getMessage());
+ }
+
+ @Override
+ public void handleException(String message, Throwable exception) {
+ reported.add(message);
+ }
+
+ @Override
+ public void handleException(String message, Exchange exchange,
Throwable exception) {
+ reported.add(message);
+ }
+ });
+ Storage storage = endpoint.getStorageClient();
+ List<Blob> blobs = new ArrayList<>();
+ for (String name : List.of("a.txt", "b/../c.txt", "d.txt")) {
+ blobs.add(storage.create(BlobInfo.newBuilder("myRejectBucket",
name).build(),
+ name.getBytes(StandardCharsets.UTF_8)));
+ }
+
+ Queue<Exchange> exchanges = consumer.createExchanges(blobs);
+
+ assertThat(exchanges)
+ .extracting(e ->
e.getMessage().getHeader(GoogleCloudStorageConstants.OBJECT_NAME, String.class))
+ .containsExactly("a.txt", "d.txt");
+ assertThat(reported).hasSize(1);
+ assertThat(reported.get(0)).contains("b/../c.txt");
+ }
+
+ @Test
+ void fullyDynamicExpressionResolvingToAnAbsoluteDirectoryKeepsWorking()
throws Exception {
+ // the directory comes from the route author's own expression, so an
absolute result is not confined
+ String directory = new File(DOWNLOAD_DIR).getAbsolutePath();
+ String expression = "${header.dir}/${file:name}";
GoogleCloudStorageConsumer consumer = createConsumer(DOWNLOAD_DIR);
Exchange exchange = new DefaultExchange(context);
+ exchange.getMessage().setHeader("dir", directory);
- assertThat(consumer.evaluateFileExpression(exchange, expression,
"../escape.txt"))
- .isEqualTo("target/../escape.txt");
+ assertThat(consumer.evaluateFileExpression(exchange, expression,
"nested/file.txt"))
+ .isEqualTo(directory + "/nested/file.txt");
}
}
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 167f9263e9df..cecefd887511 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
@@ -23,6 +23,7 @@ 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.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
import static
org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
@@ -86,6 +87,99 @@ class GoogleCloudStorageFileNameHelperTest {
DIR + "-evil/file.txt",
"../gcs-download-evil/file.txt"));
}
+ @Test
+ void staticDirectoryPrefixIsTheDirectoryBeforeTheExpression() {
+
assertThat(GoogleCloudStorageFileNameHelper.staticDirectoryPrefix("/tmp/downloads/${file:name}"))
+ .isEqualTo("/tmp/downloads");
+ }
+
+ @Test
+ void staticDirectoryPrefixIsTrimmedBackToARealDirectory() {
+ // /tmp/down is a partial path segment, not a directory, so it must
not be used as the containment directory
+
assertThat(GoogleCloudStorageFileNameHelper.staticDirectoryPrefix("/tmp/down${file:name}"))
+ .isEqualTo("/tmp");
+ }
+
+ @Test
+ void staticDirectoryPrefixCanBeTheFilesystemRoot() {
+
assertThat(GoogleCloudStorageFileNameHelper.staticDirectoryPrefix("/${file:name}"))
+ .isEqualTo("/");
+ }
+
+ @Test
+ void staticDirectoryPrefixOfAFullyDynamicValueIsEmpty() {
+
assertThat(GoogleCloudStorageFileNameHelper.staticDirectoryPrefix("${file:name}")).isEmpty();
+
assertThat(GoogleCloudStorageFileNameHelper.staticDirectoryPrefix("${header.dir}/file.txt")).isEmpty();
+
assertThat(GoogleCloudStorageFileNameHelper.staticDirectoryPrefix("prefix-${file:name}")).isEmpty();
+ }
+
+ @Test
+ void staticDirectoryPrefixStopsAtTheFirstExpressionToken() {
+
assertThat(GoogleCloudStorageFileNameHelper.staticDirectoryPrefix("target/a/${header.dir}/${file:name}"))
+ .isEqualTo("target/a");
+ }
+
+ @Test
+ void staticDirectoryPrefixKeepsTheSeparatorOfAWindowsDriveRoot() {
+ // C: alone is drive-relative (the current directory on that drive),
not the root
+
assertThat(GoogleCloudStorageFileNameHelper.staticDirectoryPrefix("C:\\${file:name}")).isEqualTo("C:\\");
+
assertThat(GoogleCloudStorageFileNameHelper.staticDirectoryPrefix("C:/${file:name}")).isEqualTo("C:/");
+
assertThat(GoogleCloudStorageFileNameHelper.staticDirectoryPrefix("C:/data/${file:name}")).isEqualTo("C:/data");
+ }
+
+ @Test
+ void relativeObjectNamesAreSafe() {
+ assertThatCode(() -> {
+ GoogleCloudStorageFileNameHelper.assertSafeObjectName("file.txt");
+ GoogleCloudStorageFileNameHelper.assertSafeObjectName("a/b/c.txt");
+ GoogleCloudStorageFileNameHelper.assertSafeObjectName("a/./b.txt");
+ // .. only matters as a whole path segment
+ GoogleCloudStorageFileNameHelper.assertSafeObjectName("..hidden");
+ GoogleCloudStorageFileNameHelper.assertSafeObjectName("a..b/c..");
+ }).doesNotThrowAnyException();
+ }
+
+ @Test
+ void objectNamesWithAParentSegmentAreNotSafe() {
+ for (String objectName : new String[] { "..", "../x", "a/../b.txt",
"a/..", "a\\..\\b.txt" }) {
+ assertThatIllegalArgumentException()
+ .isThrownBy(() ->
GoogleCloudStorageFileNameHelper.assertSafeObjectName(objectName))
+ .withMessageContaining(objectName)
+ .withMessageContaining("'..' path segment");
+ }
+ }
+
+ @Test
+ void absoluteObjectNamesAreNotSafe() {
+ for (String objectName : new String[] { "/etc/passwd", "\\x",
"\\\\server\\share\\x", "C:\\x", "C:x", "c:/x" }) {
+ assertThatIllegalArgumentException()
+ .isThrownBy(() ->
GoogleCloudStorageFileNameHelper.assertSafeObjectName(objectName))
+ .withMessageContaining(objectName)
+ .withMessageContaining("absolute path");
+ }
+ }
+
+ @Test
+ void relativePathInsideTheWorkingDirectoryIsAccepted() {
+ assertThatCode(() -> {
+
GoogleCloudStorageFileNameHelper.assertWithinWorkingDirectory("file.txt",
"file.txt");
+
GoogleCloudStorageFileNameHelper.assertWithinWorkingDirectory("a/../b.txt",
"b.txt");
+
GoogleCloudStorageFileNameHelper.assertWithinWorkingDirectory("/abs/dir/file.txt",
"file.txt");
+ }).doesNotThrowAnyException();
+ }
+
+ @Test
+ void relativePathClimbingOutOfTheWorkingDirectoryIsRejected() {
+ assertThatIllegalArgumentException()
+ .isThrownBy(() ->
GoogleCloudStorageFileNameHelper.assertWithinWorkingDirectory("../file.txt",
+ "./file.txt"))
+ .withMessageContaining("./file.txt")
+ .withMessageContaining("working directory");
+ assertThatIllegalArgumentException()
+ .isThrownBy(() ->
GoogleCloudStorageFileNameHelper.assertWithinWorkingDirectory("a/../../file.txt",
+ "file.txt"));
+ }
+
@Test
void symbolicLinkResolvingOutsideDirectoryIsRejected(@TempDir Path parent)
throws IOException {
Path downloadDir = Files.createDirectory(parent.resolve("downloads"));
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 e9800640e408..566575043d2f 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
@@ -2983,6 +2983,22 @@ path segments before checking that the destination
remains inside that directory
symbolic link that resolves outside the configured directory are rejected.
Valid object names using `/`
as a pseudo-directory separator continue to work.
+The remote object name is untrusted input, so the consumer now also checks it
whatever `downloadFileName`
+looks like. An absolute object name (one starting with `/` or `\`, or with a
drive letter such as `C:`) and
+an object name with a `..` path segment are rejected, even when the name would
normalize back inside the
+download directory (for example `a/../b.txt`). When the consumer lists the
bucket, a rejected object is skipped
+and reported to the consumer's exception handler, which logs a warning by
default, and the other objects of the
+poll are consumed as usual. The rejected object stays in the bucket, so it is
reported again on every poll until it
+is removed or renamed. With `objectName`, which consumes a single object, the
poll fails instead, and the failure is
+reported on every poll.
+
+The containment check is now also applied when `downloadFileName` is
configured as a directory followed
+by an expression, such as `downloadFileName=/tmp/downloads/${file:name}`: the
destination must stay within
+the static directory prefix configured before the first expression token
(`/tmp/downloads` in the example).
+When `downloadFileName` is fully dynamic and has no static directory prefix
(for example
+`downloadFileName=${file:name}`), a relative destination must stay within the
working directory. An absolute
+destination that comes from the route's own expression is not confined.
+
=== camel-jt400
The endpoint syntax in the component metadata is now
`jt400:userID:password@systemName/objectPath`, which is what the
