diff options
| author |  Joey Hess <joeyh@joeyh.name> | 2017-08-17 22:11:31 -0400 |
|---|---|---|
| committer | Joey Hess <joeyh@joeyh.name> | 2017-08-17 22:11:31 -0400 |
| commit | c1aec2ae837c24b602c7a0ca3fc7b0fcad565758 (patch) | |
| tree | 6ce58560d65d4a3a6529569cfdde4c80b6b5e5bf /Assistant | |
| parent | d998fce1481a4bc7779e21cb2ff6a0fa42d72c7a (diff) | |
avoid the dashed ssh hostname class of security holes
Security fix: Disallow hostname starting with a dash, which would get
passed to ssh and be treated an option. This could be used by an attacker
who provides a crafted ssh url (for eg a git remote) to execute arbitrary
code via ssh -oProxyCommand.
No CVE has yet been assigned for this hole.
The same class of security hole recently affected git itself,
CVE-2017-1000117.
Method: Identified all places where ssh is run, by git grep '"ssh"'
Converted them all to use a SshHost, if they did not already, for
specifying the hostname.
SshHost was made a data type with a smart constructor, which rejects
hostnames starting with '-'.
Note that git-annex already contains extensive use of Utility.SafeCommand,
which fixes a similar class of problem where a filename starting with a
dash gets passed to a program which treats it as an option.
This commit was sponsored by Jochen Bartl on Patreon.
Diffstat (limited to 'Assistant')
| -rw-r--r-- | Assistant/Pairing/MakeRemote.hs | 4 | ||||
| -rw-r--r-- | Assistant/Ssh.hs | 11 |
2 files changed, 9 insertions, 6 deletions
diff --git a/Assistant/Pairing/MakeRemote.hs b/Assistant/Pairing/MakeRemote.hs index e847edd39..a97bb31f0 100644 --- a/Assistant/Pairing/MakeRemote.hs +++ b/Assistant/Pairing/MakeRemote.hs @@ -42,9 +42,9 @@ finishedLocalPairing msg keypair = do [ sshOpt "StrictHostKeyChecking" "no" , sshOpt "NumberOfPasswordPrompts" "0" , "-n" - , genSshHost (sshHostName sshdata) (sshUserName sshdata) - , "git-annex-shell -c configlist " ++ T.unpack (sshDirectory sshdata) ] + (genSshHost (sshHostName sshdata) (sshUserName sshdata)) + ("git-annex-shell -c configlist " ++ T.unpack (sshDirectory sshdata)) Nothing r <- liftAnnex $ addRemote $ makeSshRemote sshdata liftAnnex $ setRemoteCost (Remote.repo r) semiExpensiveRemoteCost diff --git a/Assistant/Ssh.hs b/Assistant/Ssh.hs index e439ecd23..fb4a39a17 100644 --- a/Assistant/Ssh.hs +++ b/Assistant/Ssh.hs @@ -14,6 +14,7 @@ import Utility.Rsync import Utility.FileMode import Utility.SshConfig import Git.Remote +import Utility.SshHost import Data.Text (Text) import qualified Data.Text as T @@ -64,8 +65,9 @@ sshOpt :: String -> String -> String sshOpt k v = concat ["-o", k, "=", v] {- user@host or host -} -genSshHost :: Text -> Maybe Text -> String -genSshHost host user = maybe "" (\v -> T.unpack v ++ "@") user ++ T.unpack host +genSshHost :: Text -> Maybe Text -> SshHost +genSshHost host user = either error id $ mkSshHost $ + maybe "" (\v -> T.unpack v ++ "@") user ++ T.unpack host {- Generates a ssh or rsync url from a SshData. -} genSshUrl :: SshData -> String @@ -119,8 +121,9 @@ genSshRepoName host dir | otherwise = makeLegalName $ host ++ "_" ++ dir {- The output of ssh, including both stdout and stderr. -} -sshTranscript :: [String] -> (Maybe String) -> IO (String, Bool) -sshTranscript opts input = processTranscript "ssh" opts input +sshTranscript :: [String] -> SshHost -> String -> (Maybe String) -> IO (String, Bool) +sshTranscript opts sshhost cmd input = processTranscript "ssh" + (opts ++ [fromSshHost sshhost, cmd]) input {- Ensure that the ssh public key doesn't include any ssh options, like - command=foo, or other weirdness. |