From c50aa21d5f1468fb54d5e8d65455446897443054 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Wed, 12 Apr 2023 12:33:17 -0400 Subject: [PATCH] init: Avoid autoenabling special remotes that have control characters in their names I'm on the fence about this. Notice that pulling from a git remote can pull branches that have escape sequences in their names. Git will display those as-is. Arguably git should try harder to avoid that. But, names of remotes are usually up to the local user, and autoenable changes that, and so it makes sense that git chooses to display control characters in names of remotes, and so autoenable needs to guard against it. Sponsored-by: Graham Spencer on Patreon --- Annex/SpecialRemote.hs | 8 +++++++- CHANGELOG | 10 ++++++---- doc/todo/terminal_escapes_in_filenames.mdwn | 3 +++ 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/Annex/SpecialRemote.hs b/Annex/SpecialRemote.hs index aab29cf564..2b23b06b5d 100644 --- a/Annex/SpecialRemote.hs +++ b/Annex/SpecialRemote.hs @@ -23,6 +23,7 @@ import Logs.Remote import Logs.Trust import qualified Types.Remote as Remote import Git.Types (RemoteName) +import Utility.SafeOutput import qualified Data.Map as M @@ -95,7 +96,12 @@ autoEnable = do Just (Sameas u') -> u' Nothing -> cu case (lookupName c, findType c) of - (Just name, Right t) -> do + -- Avoid auto-enabling when the name contains a + -- control character, because git does not avoid + -- displaying control characters in the name of a + -- remote, and an attacker could leverage + -- autoenabling it as part of an attack. + (Just name, Right t) | safeOutput name == name -> do showSideAction $ UnquotedString $ "Auto enabling special remote " ++ name dummycfg <- liftIO dummyRemoteGitConfig tryNonAsync (setup t (AutoEnable c) (Just u) Nothing c dummycfg) >>= \case diff --git a/CHANGELOG b/CHANGELOG index 6de2c496f1..649f74f4d4 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,17 +1,19 @@ git-annex (10.20230408) UNRELEASED; urgency=medium - * Many commands now quotes filenames that contain unusual characters the + * Many commands now quote filenames that contain unusual characters the same way that git does, to avoid exposing control characters to the terminal. * Support core.quotePath, which can be set to false to display utf8 characters as-is in filenames. - * Control characters in information coming from the repository or other - possible untrusted sources are filtered out of the display of many + * Control characters in non-filename data coming from the repository or + other possible untrusted sources are filtered out of the display of many commands. * find, findkeys, examinekey: When outputting to a terminal and --format - is not used, quote unusual characters. + is not used, quote unusual characters. (Similar to the behavior of GNU find.) * addurl --preserve-filename now rejects filenames that contain other control characters, besides the escape sequences it already rejected. + * init: Avoid autoenabling special remotes that have control characters + in their names. -- Joey Hess Sat, 08 Apr 2023 13:57:18 -0400 diff --git a/doc/todo/terminal_escapes_in_filenames.mdwn b/doc/todo/terminal_escapes_in_filenames.mdwn index 6fc967c3be..d06cf29d46 100644 --- a/doc/todo/terminal_escapes_in_filenames.mdwn +++ b/doc/todo/terminal_escapes_in_filenames.mdwn @@ -52,6 +52,9 @@ that, when outputting to a terminal? Also: git-annex initremote with autoenable may be able to cause a remote with a malicious name to be set up? +> Fixed this by silently skipping autoenable, which seems fine since only +> an attacker would ever try this. + Also: Any place that an exception is thrown with an attacker-controlled value. `giveup` has been made to filter out control characters, but that leaves other exceptions, including ones thrown by libraries. Catch all exceptions