GH-11095: Prevent file writing outside output directory

- Throw exception if '..' is used within the canonical directory.
- Also prevent the absolute directory paths.
- Add tests to verify directory traversal prevention.
- Removed the NOSONAR's from class

- Utilize checkFile for file creation in:
   - FileWriteingMessageHandler
   - AbstractRemoteFileOutboundGateway
- Add additional test for multiple file navigation
- Add additional test for isAbsolute filepath
- Apply changes from code review

Rename checkFile to newFileInDirectoryIfValid

- Move newFileInDirectoryIfValid to FileUtils

Refactor file validation and clean up comments

- Remove try-catch blocks since checked exceptions are removed.
- Delete Sonar suppression comments to maintain clean code.
- Remove informational comments from tests.

Refactor file validation and add traversal tests

- Simplify path resolution in FileUtils.
- Throw InvalidPathException instead of MessagingException.
- Add directory traversal tests to FileWritingMessageHandlerTests.
- Add directory traversal tests to FtpTests.
  - AbstractRemoteFileOutboundGateway does not have tests so,
    FTPTests were used as a proxy
- FTP tests showed where a space can be inserted in front of a
    directory and still be accepted.
    This was remediated by stripping blanks in front of the fileName
- Update the traversal validation code to use the canonical calls to identify changes
  It handles the navigation for all character variations, where the homegrown only checked for `..`.

newFileInDirectoryIfValid must return un altered File

Any File returned (after inspection) must contain the original filename

Refactor absolute path validation and tests

- Allow absolute paths in file validation when a directory is provided.
- Fix FileUtils to reject absolute paths only if directory is empty.
- Revert resultFile creation to avoid unintended path validation.
- Refactor synchronizer tests to use Mockito for simpler session mocking.
- Update FTP and synchronizer tests to align with new validation rules.

Reject absolute directory for file sync

- Absolute directory in file sync is reject regardles if parent directory is present.

Move return in FileUtils to outside of try block

- Add NOSONAR flags back to code
- Remove unnecessary afterPropertiesSet and setBeanFactory from test
- Resolve checkstyle error

Refactor file validation for empty directories

- Resolve absolute paths properly when target directories are empty.
- Remove leading whitespace stripping to maintain strict file paths.
- Add tests to verify path traversal logic for empty local directories.

resultFile in the handleRequestMessage must be validated

- Security scanner recommended that resultFile be validated for traversal.
- tempfile also needs to be validated because it can be used instead of
   result file in AIL, IGNORE, REPLACE, REPLACE_IF_MODIFIED  cases.

Refactor file validation and update tests

Refactor FileUtils to validate paths by comparing canonical and
absolute paths instead of checking if they start with the directory
path. This simplifies the validation logic and removes the need for
the ABSOLUTE_DIR constant.

- Remove redundant tests from AbstractRemoteFileSynchronizerTests
- Add OS-specific directory traversal tests
  These tests still fail because the filter routine considers absolute navigation
  a security issue
- Update FileWritingMessageHandler to use standard File instantiation
  This is based on our discussion that we only needed to test one of the entries
  not both
- Refactor helper methods to be static
- Set java.io.tmpdir to tmp dir when on macOS
  Java tmpdir defaults to /var however on macOS this is a symlink to /private/var
  Thus any test that tests for improper traversal of directories or uses code
  that inspects for improper traversal will fail because of the symlink
  due to how java implements its canonical resolution
- Atomically replace the destination file with the temporary file,
  ensuring data integrity by overwriting any existing version in a single,
  indivisible operation.
  Resolves Issue:  There is a Time-of-Check to Time-of-Use race condition between the
   canonical path check in FileUtils.newFileInDirectoryIfValid and the
   directory creation/file writing. A local attacker can replace a
   valid directory with a symlink during this window, leading to an
   Arbitrary File Overwrite.

Consolidate test temp dir config and update tests

- Move Mac OS X test temp dir config to root to reduce duplication.
- Split checkPathOnAbsolute FTP test for OS-specific validation.
- Add comment in FileWritingMessageHandler to clarify path checks.

Move java_tmp prop set for mac to configure method

- Remove blank lines left over from previous commit

Revert build.gradle to 7.1.x-internal

- Move property set to common configuration section

* Some code cleanup and Windows compatibility for tests

Fixes: #11095

Author: cppwfs

build.gradle

@@ -355,6 +355,11 @@ configure(javaProjects) { subproject ->
 
         jvmArgs '-Xshare:off',
 				'--enable-native-access=ALL-UNNAMED'
+
+		if (org.gradle.internal.os.OperatingSystem.current().isMacOsX()) {
+			systemProperty 'java.io.tmpdir', '/private/tmp'
+		}
+
 	}
 
     checkstyle {

spring-integration-file/src/main/java/org/springframework/integration/file/FileWritingMessageHandler.java

@@ -521,7 +521,9 @@ protected Object handleRequestMessage(Message<?> requestMessage) {
 
 		File destinationDirectoryToUse = evaluateDestinationDirectoryExpression(requestMessage);
 
-		File tempFile = new File(destinationDirectoryToUse, generatedFileName + this.temporaryFileSuffix);
+		File tempFile = FileUtils.newFileInDirectoryIfValid(destinationDirectoryToUse,
+				generatedFileName + this.temporaryFileSuffix);
+		// Validation above prevents Path Traversal for the line below; no redundant check needed.
 		File resultFile = new File(destinationDirectoryToUse, generatedFileName);
 
 		boolean exists = resultFile.exists();

spring-integration-file/src/main/java/org/springframework/integration/file/remote/gateway/AbstractRemoteFileOutboundGateway.java

@@ -51,6 +51,7 @@
 import org.springframework.integration.file.remote.session.Session;
 import org.springframework.integration.file.remote.session.SessionFactory;
 import org.springframework.integration.file.support.FileExistsMode;
+import org.springframework.integration.file.support.FileUtils;
 import org.springframework.integration.handler.AbstractReplyProducingMessageHandler;
 import org.springframework.integration.handler.ExpressionEvaluatingMessageProcessor;
 import org.springframework.integration.handler.MessageProcessor;
@@ -1163,8 +1164,9 @@ protected File get(Message<?> message, Session<F> session, String remoteDir, //
 			}
 			fileInfo = files[0];
 		}
-		final File localFile =
-				new File(generateLocalDirectory(message, remoteDir), generateLocalFileName(message, remoteFilename));
+		File localFile =
+				FileUtils.newFileInDirectoryIfValid(generateLocalDirectory(message, remoteDir),
+						generateLocalFileName(message, remoteFilename));
 		FileExistsMode existsMode = resolveFileExistsMode(message);
 		boolean appending = FileExistsMode.APPEND.equals(existsMode);
 		boolean exists = localFile.exists();

spring-integration-file/src/main/java/org/springframework/integration/file/remote/synchronizer/AbstractInboundFileSynchronizer.java

@@ -24,6 +24,8 @@
 import java.io.OutputStream;
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.nio.file.Files;
+import java.nio.file.StandardCopyOption;
 import java.util.Arrays;
 import java.util.Comparator;
 import java.util.List;
@@ -75,6 +77,7 @@
  * @author Gary Russell
  * @author Artem Bilan
  * @author Ngoc Nhan
+ * @author Glenn Renfro
  *
  * @since 2.0
  */
@@ -461,7 +464,7 @@ protected boolean copyFileToLocalDirectory(@Nullable String remoteDirectoryPath,
 
 		long modified = getModified(remoteFile);
 
-		File localFile = new File(localDirectory, localFileName);
+		File localFile = FileUtils.newFileInDirectoryIfValid(localDirectory, localFileName);
 		boolean exists = localFile.exists();
 		if (!exists || (this.preserveTimestamp && modified != localFile.lastModified())) {
 			if (!exists &&
@@ -556,11 +559,25 @@ private boolean copyRemoteContentToLocalFile(Session<F> session, String remoteFi
 					+ "' from the remote to the local directory", e);
 		}
 
-		renamed = tempFile.renameTo(localFile);
+		try {
+			Files.move(tempFile.toPath(), localFile.toPath(), StandardCopyOption.ATOMIC_MOVE,
+					StandardCopyOption.REPLACE_EXISTING);
+			renamed = true;
+		}
+		catch (IOException e) {
+			renamed = false;
+		}
 
 		if (!renamed) {
 			if (localFile.delete()) {
-				renamed = tempFile.renameTo(localFile);
+				try {
+					Files.move(tempFile.toPath(), localFile.toPath(), StandardCopyOption.ATOMIC_MOVE,
+							StandardCopyOption.REPLACE_EXISTING);
+					renamed = true;
+				}
+				catch (IOException e) {
+					renamed = false;
+				}
 				if (!renamed && this.logger.isInfoEnabled()) {
 					this.logger.info("Cannot rename '"
 							+ tempFileName

spring-integration-file/src/main/java/org/springframework/integration/file/support/FileUtils.java

@@ -16,8 +16,12 @@
 
 package org.springframework.integration.file.support;
 
+import java.io.File;
+import java.io.IOException;
+import java.io.UncheckedIOException;
 import java.lang.reflect.Array;
 import java.nio.file.FileSystems;
+import java.nio.file.InvalidPathException;
 import java.util.Arrays;
 import java.util.Comparator;
 import java.util.function.Predicate;
@@ -69,6 +73,37 @@ public static <F> F[] purgeUnwantedElements(F[] fileArray, Predicate<? extends F
 		}
 	}
 
+	/**
+	 * Create a new file instance representing the specified file within the given base directory,
+	 * ensuring that the resulting file remains within the intended target directory.
+	 * Prevent Path Traversal vulnerabilities by explicitly rejecting file names that are absolute
+	 * paths or that resolve to canonical paths outside the provided base directory.
+	 * @param directory The base directory where the file is intended to be placed.
+	 * @param fileName  The name of the file or the relative path to be resolved against the base directory.
+	 * @return A {@link File} object representing the securely resolved destination path.
+	 * @throws InvalidPathException If the {@code fileName} is an absolute path or attempts to traverse
+	 * outside the {@code directory}.
+	 * @throws UncheckedIOException If an I/O error occurs while resolving the canonical paths.
+	 * @since 5.5.21
+	 */
+	public static File newFileInDirectoryIfValid(File directory, String fileName) {
+		if (new File(fileName).isAbsolute()) {
+			throw new InvalidPathException(fileName,
+					"The file is trying to leave the target output directory of " + directory);
+		}
+		try {
+			File file = new File(directory.getCanonicalFile(), fileName);
+			if (!file.getCanonicalPath().equals(file.getAbsolutePath())) {
+				throw new InvalidPathException(fileName,
+						"The file is trying to leave the target output directory of " + directory);
+			}
+			return file;
+		}
+		catch (IOException ex) {
+			throw new UncheckedIOException(ex);
+		}
+	}
+
 	private FileUtils() {
 	}
 

spring-integration-file/src/test/java/org/springframework/integration/file/FileWritingMessageHandlerTests.java

@@ -24,6 +24,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.file.Files;
+import java.nio.file.InvalidPathException;
 import java.nio.file.attribute.PosixFilePermission;
 import java.util.Map;
 import java.util.Set;
@@ -53,6 +54,7 @@
 import org.springframework.integration.test.util.TestUtils;
 import org.springframework.messaging.Message;
 import org.springframework.messaging.MessageHandlingException;
+import org.springframework.messaging.MessagingException;
 import org.springframework.messaging.support.GenericMessage;
 import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler;
 import org.springframework.util.FileCopyUtils;
@@ -691,6 +693,19 @@ public void newFileCallback() throws Exception {
 		assertFileContentIs(result, "foo" + System.lineSeparator() + "barbar");
 	}
 
+	@Test
+	public void checkPathThrowsOnDirectoryTraversal() {
+		Message<?> message = MessageBuilder.withPayload("testdata").build();
+		DefaultFileNameGenerator fileNameGenerator = new DefaultFileNameGenerator();
+		fileNameGenerator.setExpression("'base/../../../escaped.txt'");
+		handler.setFileNameGenerator(fileNameGenerator);
+
+		assertThatExceptionOfType(MessagingException.class)
+			.isThrownBy(() -> handler.handleMessage(message))
+			.withStackTraceContaining("trying to leave the target output directory")
+			.withRootCauseInstanceOf(InvalidPathException.class);
+	}
+
 	void assertFileContentIsMatching(Message<?> result) throws IOException {
 		assertFileContentIs(result, SAMPLE_CONTENT);
 	}

spring-integration-file/src/test/java/org/springframework/integration/file/remote/synchronizer/AbstractRemoteFileSynchronizerTests.java

@@ -20,6 +20,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.nio.file.InvalidPathException;
 import java.util.LinkedList;
 import java.util.Queue;
 import java.util.UUID;
@@ -29,6 +30,8 @@
 import java.util.stream.Stream;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.EnabledOnOs;
+import org.junit.jupiter.api.condition.OS;
 import org.junit.jupiter.api.io.TempDir;
 
 import org.springframework.beans.factory.BeanFactory;
@@ -44,6 +47,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
+import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
 
 /**
@@ -273,11 +277,93 @@ protected String protocol() {
 		assertThat(localDir.list()).contains("dir1", "dir2");
 	}
 
-	private AbstractInboundFileSynchronizingMessageSource<String> createSource(AtomicInteger count) {
+	@Test
+	public void checkPathThrowsOnDirectoryTraversal(@TempDir File localDir) {
+		verifyDirectoryNotTheSame(localDir, "../base/escaped.txt");
+	}
+
+	@Test
+	public void checkPathOnDirectoryMidLineTraversal(@TempDir File localDir) {
+		verifyDirectoryNotTheSame(localDir, "base/../escaped.txt");
+	}
+
+	private static AbstractInboundFileSynchronizer<String> getAbstractInboundFileSynchronizer(String fileName) {
+		StringSession session = mock(StringSession.class);
+		given(session.list("remoteDir")).willReturn(new String[] {fileName});
+		given(session.getHostPort()).willReturn("mock:1234");
+
+		AbstractInboundFileSynchronizer<String> sync = createSynchronizerWithSession(session);
+		sync.setRemoteDirectory("remoteDir");
+		return sync;
+	}
+
+	@Test
+	public void checkPathThrowsOnDirectoryExtendMidLineTraversal(@TempDir File localDir) {
+		verifyDirectoryNotTheSame(localDir, "base/subdir/../../../../escaped.txt");
+	}
+
+	@Test
+	@EnabledOnOs(OS.WINDOWS)
+	void checkPathThrowsOnAbsoluteDirectoryTraversalLinuxMacWindows(@TempDir File localDir) {
+		verifyDirectoryNotTheSame(localDir, "C:\\projects\\data\\file.txt");
+	}
+
+	@Test
+	@EnabledOnOs(OS.WINDOWS)
+	void notTraversalOnCurrentDirWindows() {
+		AbstractInboundFileSynchronizer<String> sync = getAbstractInboundFileSynchronizer("build\\tmp\\file.txt");
+
+		sync.synchronizeToLocalDirectory(new File(""));
+
+		assertThat(new File("build\\tmp\\file.txt")).exists();
+	}
+
+	@Test
+	@EnabledOnOs({OS.LINUX, OS.MAC})
+	void checkPathThrowsOnAbsoluteDirectoryTraversalLinuxMac(@TempDir File localDir) {
+		verifyDirectoryNotTheSame(localDir, "/base/escaped.txt");
+	}
+
+	private static void verifyDirectoryNotTheSame(File localDir, String fileName) {
+		AbstractInboundFileSynchronizer<String> sync = getAbstractInboundFileSynchronizer(fileName);
+
+		assertThatExceptionOfType(MessagingException.class)
+				.isThrownBy(() -> sync.synchronizeToLocalDirectory(localDir))
+				.withStackTraceContaining("trying to leave the target output directory")
+				.withRootCauseInstanceOf(InvalidPathException.class);
+	}
+
+	private static AbstractInboundFileSynchronizer<String> createSynchronizerWithSession(StringSession session) {
+		return new AbstractInboundFileSynchronizer<>(() -> session) {
+
+			@Override
+			protected boolean isFile(String file) {
+				return true;
+			}
+
+			@Override
+			protected String getFilename(String file) {
+				return file;
+			}
+
+			@Override
+			protected long getModified(String file) {
+				return 0;
+			}
+
+			@Override
+			protected String protocol() {
+				return "mock";
+			}
+
+		};
+	}
+
+	private static AbstractInboundFileSynchronizingMessageSource<String> createSource(AtomicInteger count) {
 		return createSource(createLimitingSynchronizer(count));
 	}
 
-	private AbstractInboundFileSynchronizingMessageSource<String> createSource(
+	private static AbstractInboundFileSynchronizingMessageSource<String> createSource(
 			AbstractInboundFileSynchronizer<String> sync) {
 
 		AbstractInboundFileSynchronizingMessageSource<String> source =
@@ -297,7 +383,7 @@ public String getComponentType() {
 		return source;
 	}
 
-	private AbstractInboundFileSynchronizer<String> createLimitingSynchronizer(final AtomicInteger count) {
+	private static AbstractInboundFileSynchronizer<String> createLimitingSynchronizer(final AtomicInteger count) {
 		SessionFactory<String> sf = new StringSessionFactory();
 		AbstractInboundFileSynchronizer<String> sync = new AbstractInboundFileSynchronizer<>(sf) {
 
@@ -423,7 +509,7 @@ public Object getClientInstance() {
 
 		@Override
 		public String getHostPort() {
-			return "mock:6666";
+			return "mock:1234";
 		}
 
 	}