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

diff --git a/Annex/SpecialRemote.hs b/Annex/SpecialRemote.hs
index aab29cf564d5c17d0c953b4a8f19d69de3ed04ea..2b23b06b5ded2ce0f832acbd449f1812ace3da58 100644 (file)
--- 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 6de2c496f117f0d1ac76746e143db12cc1fa017d..649f74f4d40c4d878d4f205b71470d1cddb94dbd 100644 (file)
--- 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 <id@joeyh.name>  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 6fc967c3be7435e7e3b05bb3b2561fe78d7f7327..d06cf29d461321fc703af77473ea57f499bfd840 100644 (file)
--- 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