NIFI-15653 Fix DeleteSFTP security check for dot directory path#11266
Open
rakesh-rsky wants to merge 1 commit into
Open
NIFI-15653 Fix DeleteSFTP security check for dot directory path#11266rakesh-rsky wants to merge 1 commit into
rakesh-rsky wants to merge 1 commit into
Conversation
When the Remote Path property is set to '.' (as produced by ListSFTP),
Paths.get('.').normalize() returns an empty Path (''). Resolving a
filename against that empty path yields a single-component relative
path whose getParent() returns null, causing the directory-traversal
security check to fail incorrectly and route every FlowFile to the
failure relationship.
Fix: when fileParent is null (single-component relative path), substitute
Paths.get('') for the comparison. Both the empty normalized path and a
null parent represent the implicit current directory, so the check now
correctly passes. Path-traversal attempts (e.g. '../etc/passwd') still
produce a non-empty parent that does not equal the empty directoryPath,
so the security guard remains intact.
Add TestDeleteSFTP.deletesFileWhenDirectoryPathIsDot() to cover the
regression.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When the Remote Path property is configured as . (dot) — the default produced by ListSFTP — DeleteSFTP incorrectly routes every FlowFile to the failure relationship instead of deleting the file.
Root Cause
`java
final Path directoryPath = Paths.get(directoryPathProperty).normalize(); // "." → ""
final Path filePath = directoryPath.resolve(filename).normalize(); // "" + "test.txt" → "test.txt"
if (!directoryPath.equals(filePath.getParent())) { // "".equals(null) → false ← bug
`
Paths.get(".").normalize() returns an empty Path (""). Resolving a filename against that path yields a single-component relative path whose getParent() returns
ull. The check Paths.get("").equals(null) is always false, so the security guard fires incorrectly for every valid file.
Fix
java final Path fileParent = filePath.getParent(); if (!directoryPath.equals(fileParent == null ? Paths.get("") : fileParent)) {When fileParent is
ull (single-component relative path), substitute Paths.get("") — the same empty-path value that directoryPath holds. Both represent the implicit current directory, so the check now passes correctly.
Path-traversal attempts (e.g. ../etc/passwd) still produce a non-empty parent that does not equal the empty directoryPath, so the security guard remains fully intact.
Testing
Added TestDeleteSFTP.deletesFileWhenDirectoryPathIsDot() using the existing embedded SSH server to confirm that a file in the root of the SFTP server is successfully deleted when the directory property is set to .(dot).