Repository navigation
restrict SftpStreamProxy target host to host name characters - #793
Open
rootvector2 wants to merge 1 commit into
Open
rootvector2 wants to merge 1 commit into
rootvector2 wants to merge 1 commit into
Conversation
SftpStreamProxy.connect formats the host of the sftp URI into the command it runs on the proxy host, so shell syntax in the host name was executed there. Refuse a target host that is not a host name or an IP address literal before the proxy session is opened.
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
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.
SftpStreamProxy.connectformats the target host into the command it runs on the proxy host, andHostFileNameParseronly ends a host name at/ ; ? : @ & = + $ ,, so everything else in the URI authority reaches the proxy's shell. With a stream proxy configured, resolvingsftp://user@target|id/runsnc -q 0 target|id 22there, and backticks, spaces, newlines, redirections, quotes and a leading-pass the same way: OS command injection on the proxy host for an application that resolves a URI whose host it does not choose, the class of CVE-2023-51385 in OpenSSH'sProxyCommand. Found while checking where URI components end up in a command line.connectnow refuses a target host that starts with-or holds anything but letters, digits and- . _ : % [ ], before it opens the proxy session. The check sits next to theString.formatcall because that is the one place a URI value is handed to a shell, so it holds for every command format. Host names, IPv4 addresses and bracketed IPv6 literals with a zone id pass as before; the newSftpStreamProxyTestrecords the command an embedded SSH server receives and fails on the current code.mvn; that'smvnon the command line by itself. The default goal ran on every module with rat, japicmp, javadoc, spotbugs, pmd and checkstyle clean and no test failures, but not error free: the local HTTP provider tests cannot parse the URI built from my machine's host name (unknown_5e:ad:3d:f7:97:a6), and they fail the same way without this change.