This is an automated email from the ASF dual-hosted git repository.
oscerd 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 177fb1fdff90 CAMEL-24487: remote-file consumers - contain remote
operations within the configured directory (#25730)
177fb1fdff90 is described below
commit 177fb1fdff90632ebfd27772a625b720c1cab3e7
Author: Andrea Cosentino <[email protected]>
AuthorDate: Tue Aug 25 23:33:31 2026 +0200
CAMEL-24487: remote-file consumers - contain remote operations within the
configured directory (#25730)
The remote-file consumers build the absolute remote path by concatenating
the
configured directory with the file name reported in the server directory
listing,
and then use that result as the operand for the subsequent retrieve, delete
and
rename operations. No lexical normalization or containment check was
applied at any
point between the listing and those operations, so a listing entry that is
not a
single path segment could resolve outside the configured directory.
This is inconsistent with the file producer and with the localWorkDirectory
download
path, which both contain the resolved path via FileUtil.compactPath plus a
path-boundary check when jailStartingDirectory is enabled (CAMEL-23765,
CAMEL-23868).
Adds a separator-aware GenericFileHelper.isWithinDirectory overload, since
remote
paths always use '/' regardless of the platform Camel runs on, and a
GenericFileConsumer.isWithinStartingDirectory strategy that the consumer
consults in
isValidFile when jailStartingDirectory is enabled. The base implementation
is a no-op
for local directory listings, whose names are always single path segments;
RemoteFileConsumer (camel-ftp, camel-ftps, camel-sftp, camel-mina-sftp,
camel-azure-files) and SmbConsumer (camel-smb) override it to compact the
resolved
path and check it still resolves inside the directory being polled. A file
resolving
outside is skipped with a warning, so the rest of the listing still
processes.
The check runs after the existing match filters, so the "." and ".."
listing entries
that SFTP servers return are already excluded by the hidden-file rule and
do not log.
Adds GenericFileHelper tests for the separator-aware overload,
RemoteFileConsumerStartingDirectoryJailTest covering FTP and SFTP
endpoints, and a
4.23 upgrade-guide note.
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
.../camel/component/file/GenericFileConsumer.java | 23 +++++++
.../camel/component/file/GenericFileHelper.java | 24 ++++++-
.../component/file/GenericFileHelperTest.java | 27 ++++++++
.../component/file/remote/RemoteFileConsumer.java | 23 +++++++
...emoteFileConsumerStartingDirectoryJailTest.java | 77 ++++++++++++++++++++++
.../apache/camel/component/smb/SmbConsumer.java | 21 ++++++
.../ROOT/pages/camel-4x-upgrade-guide-4_23.adoc | 17 +++++
7 files changed, 210 insertions(+), 2 deletions(-)
diff --git
a/components/camel-file/src/main/java/org/apache/camel/component/file/GenericFileConsumer.java
b/components/camel-file/src/main/java/org/apache/camel/component/file/GenericFileConsumer.java
index 76a775a91b74..fe72a23124b1 100644
---
a/components/camel-file/src/main/java/org/apache/camel/component/file/GenericFileConsumer.java
+++
b/components/camel-file/src/main/java/org/apache/camel/component/file/GenericFileConsumer.java
@@ -630,6 +630,13 @@ public abstract class GenericFileConsumer<T> extends
ScheduledBatchPollingConsum
return false;
}
+ // the resolved path must stay within the starting directory, as the
name it was built from is
+ // reported by the remote server and is not necessarily a single path
segment
+ if (endpoint.isJailStartingDirectory() &&
!isWithinStartingDirectory(absoluteFilePath)) {
+ LOG.warn("Skipping file as it resolves outside the starting
directory: {}", absoluteFilePath);
+ return false;
+ }
+
// directory is always valid
if (isDirectory) {
return true;
@@ -679,6 +686,22 @@ public abstract class GenericFileConsumer<T> extends
ScheduledBatchPollingConsum
return answer;
}
+ /**
+ * Strategy to determine whether the resolved path of a listed file stays
within the configured starting directory.
+ * <p/>
+ * Consumers that build the path from a name supplied by a remote server
must override this, as such a name is not
+ * guaranteed to be a single path segment and can otherwise navigate
outside the directory being polled. The check
+ * is only consulted when {@link
GenericFileEndpoint#isJailStartingDirectory()} is enabled.
+ *
+ * @param absoluteFilePath the resolved absolute path of the listed file
+ * @return {@code true} if the path stays within the
starting directory
+ */
+ protected boolean isWithinStartingDirectory(String absoluteFilePath) {
+ // names obtained from a local directory listing are always single
path segments, so there is no
+ // boundary to enforce here
+ return true;
+ }
+
/**
* Strategy to perform hidden file matching based on endpoint
configuration.
* <p/>
diff --git
a/components/camel-file/src/main/java/org/apache/camel/component/file/GenericFileHelper.java
b/components/camel-file/src/main/java/org/apache/camel/component/file/GenericFileHelper.java
index 7c73d148ac67..ff3957e296e5 100644
---
a/components/camel-file/src/main/java/org/apache/camel/component/file/GenericFileHelper.java
+++
b/components/camel-file/src/main/java/org/apache/camel/component/file/GenericFileHelper.java
@@ -58,8 +58,28 @@ public final class GenericFileHelper {
* @param compactTarget the compacted target path (see {@link
FileUtil#compactPath(String)})
* @param compactDir the compacted directory the target must stay
within
* @return {@code true} if the target is the directory
itself or a path inside it
+ * @see #isWithinDirectory(String, String, char)
*/
public static boolean isWithinDirectory(String compactTarget, String
compactDir) {
+ return isWithinDirectory(compactTarget, compactDir,
File.separatorChar);
+ }
+
+ /**
+ * Determines whether a compacted target path is contained within a
compacted directory path, using the given path
+ * separator. Remote file paths always use {@code /} regardless of the
platform Camel runs on, so remote callers
+ * must pass {@code '/'} rather than relying on {@link File#separatorChar}.
+ *
+ * @param compactTarget the compacted target path (see {@link
FileUtil#compactPath(String, char)})
+ * @param compactDir the compacted directory the target must stay
within
+ * @param separator the path separator both paths are expressed with
+ * @return {@code true} if the target is the directory
itself or a path inside it
+ */
+ public static boolean isWithinDirectory(String compactTarget, String
compactDir, char separator) {
+ // a target that still resolves upwards after compaction escapes any
root, even when no boundary is
+ // configured, so it is never contained
+ if (compactTarget.equals("..") || compactTarget.startsWith(".." +
separator)) {
+ return false;
+ }
if (compactDir.isEmpty()) {
// no directory boundary configured
return true;
@@ -67,10 +87,10 @@ public final class GenericFileHelper {
// drop a trailing separator (if any) so the boundary comparison is
exact, regardless of whether the
// directory path was supplied with or without one
String dir = compactDir;
- if (dir.charAt(dir.length() - 1) == File.separatorChar) {
+ if (dir.charAt(dir.length() - 1) == separator) {
dir = dir.substring(0, dir.length() - 1);
}
- return compactTarget.equals(dir) || compactTarget.startsWith(dir +
File.separator);
+ return compactTarget.equals(dir) || compactTarget.startsWith(dir +
separator);
}
public static String asExclusiveReadLockKey(GenericFile file, String key) {
diff --git
a/components/camel-file/src/test/java/org/apache/camel/component/file/GenericFileHelperTest.java
b/components/camel-file/src/test/java/org/apache/camel/component/file/GenericFileHelperTest.java
index e52a63ffa497..2b6c29d10b79 100644
---
a/components/camel-file/src/test/java/org/apache/camel/component/file/GenericFileHelperTest.java
+++
b/components/camel-file/src/test/java/org/apache/camel/component/file/GenericFileHelperTest.java
@@ -71,4 +71,31 @@ public class GenericFileHelperTest {
// an empty directory imposes no boundary
assertTrue(GenericFileHelper.isWithinDirectory("anything.txt", ""));
}
+
+ @Test
+ public void isWithinDirectoryUsesTheGivenSeparator() {
+ // remote paths always use '/', regardless of the platform Camel runs
on
+ assertTrue(GenericFileHelper.isWithinDirectory("poll/file.txt",
"poll", '/'));
+ assertTrue(GenericFileHelper.isWithinDirectory("poll/sub/file.txt",
"poll", '/'));
+ assertTrue(GenericFileHelper.isWithinDirectory("poll", "poll", '/'));
+ assertTrue(GenericFileHelper.isWithinDirectory("/poll/file.txt",
"/poll", '/'));
+
+ // a trailing separator on the directory is tolerated
+ assertTrue(GenericFileHelper.isWithinDirectory("poll/file.txt",
"poll/", '/'));
+
+ // a sibling whose name merely extends the directory name is NOT
contained
+ assertFalse(GenericFileHelper.isWithinDirectory("pollute/file.txt",
"poll", '/'));
+ }
+
+ @Test
+ public void isWithinDirectoryRejectsPathsResolvingOutsideTheDirectory() {
+ // the compacted result of a listing name that navigates above the
polled directory
+ assertFalse(GenericFileHelper.isWithinDirectory("../secret.txt",
"poll", '/'));
+ assertFalse(GenericFileHelper.isWithinDirectory("../../etc/shadow",
"poll", '/'));
+ assertFalse(GenericFileHelper.isWithinDirectory("/secret.txt",
"/poll", '/'));
+
+ // a target that still resolves upwards escapes even when no directory
boundary is configured
+ assertFalse(GenericFileHelper.isWithinDirectory("..", "", '/'));
+ assertFalse(GenericFileHelper.isWithinDirectory("../secret.txt", "",
'/'));
+ }
}
diff --git
a/components/camel-ftp-common/src/main/java/org/apache/camel/component/file/remote/RemoteFileConsumer.java
b/components/camel-ftp-common/src/main/java/org/apache/camel/component/file/remote/RemoteFileConsumer.java
index 967e00c0e227..ad780f979043 100644
---
a/components/camel-ftp-common/src/main/java/org/apache/camel/component/file/remote/RemoteFileConsumer.java
+++
b/components/camel-ftp-common/src/main/java/org/apache/camel/component/file/remote/RemoteFileConsumer.java
@@ -24,9 +24,11 @@ import org.apache.camel.Ordered;
import org.apache.camel.Processor;
import org.apache.camel.component.file.GenericFile;
import org.apache.camel.component.file.GenericFileConsumer;
+import org.apache.camel.component.file.GenericFileHelper;
import org.apache.camel.component.file.GenericFileOperationFailedException;
import org.apache.camel.component.file.GenericFileProcessStrategy;
import org.apache.camel.support.SynchronizationAdapter;
+import org.apache.camel.util.FileUtil;
import org.apache.camel.util.ObjectHelper;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -38,6 +40,11 @@ public abstract class RemoteFileConsumer<T> extends
GenericFileConsumer<T> {
private static final Logger LOG =
LoggerFactory.getLogger(RemoteFileConsumer.class);
+ /**
+ * Remote paths always use {@code /}, regardless of the platform Camel
runs on.
+ */
+ private static final char REMOTE_PATH_SEPARATOR = '/';
+
protected transient boolean loggedIn;
protected transient boolean loggedInWarning;
protected transient boolean autoCreatedDone;
@@ -60,6 +67,22 @@ public abstract class RemoteFileConsumer<T> extends
GenericFileConsumer<T> {
return (RemoteFileOperations<T>) operations;
}
+ /**
+ * The file name comes from the directory listing returned by the remote
server and is not guaranteed to be a single
+ * path segment, so the path resolved from it is compacted and checked to
still be inside the directory being polled
+ * before the file is accepted for retrieval, deletion or renaming.
+ */
+ @Override
+ protected boolean isWithinStartingDirectory(String absoluteFilePath) {
+ if (absoluteFilePath == null) {
+ return false;
+ }
+ String directory = getEndpoint().getConfiguration().getDirectory();
+ String compactDir = directory != null ?
FileUtil.compactPath(directory, REMOTE_PATH_SEPARATOR) : "";
+ return GenericFileHelper.isWithinDirectory(
+ FileUtil.compactPath(absoluteFilePath, REMOTE_PATH_SEPARATOR),
compactDir, REMOTE_PATH_SEPARATOR);
+ }
+
@Override
protected Exchange createExchange(GenericFile<T> file) {
Exchange answer = createExchange(true);
diff --git
a/components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/RemoteFileConsumerStartingDirectoryJailTest.java
b/components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/RemoteFileConsumerStartingDirectoryJailTest.java
new file mode 100644
index 000000000000..c4a8008a597c
--- /dev/null
+++
b/components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/RemoteFileConsumerStartingDirectoryJailTest.java
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.component.file.remote;
+
+import org.apache.camel.test.junit6.CamelTestSupport;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * The file name used to build the path of a polled file comes from the
directory listing returned by the remote server,
+ * so it is not guaranteed to be a single path segment. Verifies the resolved
path is kept inside the directory being
+ * polled before it is used as the operand for retrieving, deleting or
renaming.
+ */
+class RemoteFileConsumerStartingDirectoryJailTest extends CamelTestSupport {
+
+ private RemoteFileConsumer<?> consumer(String uri) throws Exception {
+ RemoteFileEndpoint<?> endpoint = context.getEndpoint(uri,
RemoteFileEndpoint.class);
+ return (RemoteFileConsumer<?>) endpoint.createConsumer(exchange -> {
+ });
+ }
+
+ @Test
+ void shouldAcceptPathsWithinTheStartingDirectory() throws Exception {
+ RemoteFileConsumer<?> consumer = consumer("ftp://hostname/poll";);
+
+ assertTrue(consumer.isWithinStartingDirectory("poll/file.txt"));
+ assertTrue(consumer.isWithinStartingDirectory("poll/sub/file.txt"));
+ // a ../ that still resolves back inside the polled directory is
legitimate
+ assertTrue(consumer.isWithinStartingDirectory("poll/sub/../file.txt"));
+ }
+
+ @Test
+ void shouldRejectPathsEscapingTheStartingDirectory() throws Exception {
+ RemoteFileConsumer<?> consumer = consumer("ftp://hostname/poll";);
+
+
assertFalse(consumer.isWithinStartingDirectory("poll/a/../../../secret.txt"));
+ assertFalse(consumer.isWithinStartingDirectory("poll/../secret.txt"));
+
assertFalse(consumer.isWithinStartingDirectory("poll/../../etc/shadow"));
+ // a sibling directory whose name merely extends the polled directory
name is not contained
+
assertFalse(consumer.isWithinStartingDirectory("poll/../pollute/secret.txt"));
+ assertFalse(consumer.isWithinStartingDirectory(null));
+ }
+
+ @Test
+ void shouldRejectPathsEscapingTheStartingDirectoryOverSftp() throws
Exception {
+ RemoteFileConsumer<?> consumer = consumer("sftp://hostname/poll";);
+
+ assertTrue(consumer.isWithinStartingDirectory("poll/file.txt"));
+
assertFalse(consumer.isWithinStartingDirectory("poll/a/../../../secret.txt"));
+ assertFalse(consumer.isWithinStartingDirectory("poll/../secret.txt"));
+ }
+
+ @Test
+ void shouldRejectUpwardsPathsWhenPollingTheSessionRoot() throws Exception {
+ RemoteFileConsumer<?> consumer = consumer("ftp://hostname";);
+
+ assertTrue(consumer.isWithinStartingDirectory("file.txt"));
+ // no directory is configured, but navigating above the session root
still escapes
+ assertFalse(consumer.isWithinStartingDirectory("../secret.txt"));
+ }
+}
diff --git
a/components/camel-smb/src/main/java/org/apache/camel/component/smb/SmbConsumer.java
b/components/camel-smb/src/main/java/org/apache/camel/component/smb/SmbConsumer.java
index 4ffa70057271..c529b5502783 100644
---
a/components/camel-smb/src/main/java/org/apache/camel/component/smb/SmbConsumer.java
+++
b/components/camel-smb/src/main/java/org/apache/camel/component/smb/SmbConsumer.java
@@ -32,6 +32,7 @@ import org.apache.camel.Processor;
import org.apache.camel.component.file.GenericFile;
import org.apache.camel.component.file.GenericFileConsumer;
import org.apache.camel.component.file.GenericFileEndpoint;
+import org.apache.camel.component.file.GenericFileHelper;
import org.apache.camel.component.file.GenericFileOperationFailedException;
import org.apache.camel.component.file.GenericFileOperations;
import org.apache.camel.component.file.GenericFileProcessStrategy;
@@ -47,6 +48,11 @@ public class SmbConsumer extends
GenericFileConsumer<FileIdBothDirectoryInformat
private static final Logger LOG =
LoggerFactory.getLogger(SmbConsumer.class);
+ /**
+ * Remote paths always use {@code /}, regardless of the platform Camel
runs on.
+ */
+ private static final char REMOTE_PATH_SEPARATOR = '/';
+
private final SmbEndpoint endpoint;
private final SmbConfiguration configuration;
private final String endpointPath;
@@ -71,6 +77,21 @@ public class SmbConsumer extends
GenericFileConsumer<FileIdBothDirectoryInformat
return (GenericFileEndpoint<FileIdBothDirectoryInformation>)
super.getEndpoint();
}
+ /**
+ * The file name comes from the directory listing returned by the remote
share and is not guaranteed to be a single
+ * path segment, so the path resolved from it is compacted and checked to
still be inside the directory being polled
+ * before the file is accepted for retrieval, deletion or renaming.
+ */
+ @Override
+ protected boolean isWithinStartingDirectory(String absoluteFilePath) {
+ if (absoluteFilePath == null) {
+ return false;
+ }
+ return GenericFileHelper.isWithinDirectory(
+ FileUtil.compactPath(absoluteFilePath, REMOTE_PATH_SEPARATOR),
+ FileUtil.compactPath(endpointPath, REMOTE_PATH_SEPARATOR),
REMOTE_PATH_SEPARATOR);
+ }
+
@Override
protected boolean pollDirectory(
Exchange dynamic, String path,
List<GenericFile<FileIdBothDirectoryInformation>> fileList, int depth) {
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 88c74de6f50b..3c88f2fbad71 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
@@ -248,3 +248,20 @@ Camel then no longer controls how the payload is read.
The legacy `sse` `transportType` has been removed. It follows the support
removal in
https://github.com/langchain4j/langchain4j/commit/7f4e99fd637137ab9d3bf116eb62047ac757a818[langchain4j-core
1.19.0].
+
+=== camel-ftp, camel-sftp, camel-ftps, camel-mina-sftp, camel-azure-files,
camel-smb
+
+The remote-file consumers now ensure the path resolved for a polled file stays
within the directory being
+polled. The file name that path is built from is reported by the remote server
in its directory listing and is
+not guaranteed to be a single path segment, so a listing entry containing
`../` sequences could previously
+resolve to a path outside the configured directory and be used as the operand
for retrieving, deleting or
+renaming a file.
+
+The containment check honours the existing `jailStartingDirectory` option
(default `true`), consistent with the
+file producer and with the `localWorkDirectory` download path; set
`jailStartingDirectory=false` to disable it.
+A file that resolves outside the configured directory is now skipped, and a
warning is logged.
+
+Ordinary listings are unaffected, as a listed name is normally a single path
segment, and a `../` that still
+resolves back inside the polled directory remains accepted. Two configurations
can newly see files skipped: a
+server that reports names navigating above the polled directory, and a
`fileName` expression (used when
+`useList=false`) that navigates above it. Set `jailStartingDirectory=false` if
such a path is intended.
