From ed39979ac8ec6ff2a279939440574927d5cc894a Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Fri, 19 Aug 2022 13:31:16 -0400 Subject: [PATCH] import: Avoid following symbolic links inside directories being imported Too big a footgun. This does not prevent attackers who can write to the directory being imported from racing the check. But they can cause anything to be imported anyway, so would be limited to getting the legacy import to follow into a directory they do not write to, and move files out of it into the annex. (The directory special remote does not have that problem since it does not move files.) Sponsored-by: Jack Hill on Patreon --- CHANGELOG | 2 ++ CmdLine/Seek.hs | 2 +- Remote/Directory.hs | 2 +- .../git-annex-import_imports_outside_of_directory.mdwn | 2 ++ doc/git-annex-import.mdwn | 8 +++++++- 5 files changed, 13 insertions(+), 3 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index b3d274a8cc..0c600efc39 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -25,6 +25,8 @@ git-annex (10.20220725) UNRELEASED; urgency=medium annex.security.allowed-ip-addresses is configured to allow using curl. * Fix a regression in 10.20220624 that caused git-annex add to crash when there was an unstaged deletion. + * import: Avoid following symbolic links inside directories being + imported. -- Joey Hess Mon, 25 Jul 2022 15:35:45 -0400 diff --git a/CmdLine/Seek.hs b/CmdLine/Seek.hs index 600a62ee5e..4381ee84d3 100644 --- a/CmdLine/Seek.hs +++ b/CmdLine/Seek.hs @@ -113,7 +113,7 @@ withPathContents a params = do ( map (\f -> let f' = toRawFilePath f in (f', P.makeRelative (P.takeDirectory (P.dropTrailingPathSeparator p')) f')) - <$> dirContentsRecursiveSkipping (".git" `isSuffixOf`) True p + <$> dirContentsRecursiveSkipping (".git" `isSuffixOf`) False p , return [(p', P.takeFileName p')] ) where diff --git a/Remote/Directory.hs b/Remote/Directory.hs index 8220b57dda..2a5cfa2766 100644 --- a/Remote/Directory.hs +++ b/Remote/Directory.hs @@ -363,7 +363,7 @@ removeExportLocation topdir loc = listImportableContentsM :: IgnoreInodes -> RawFilePath -> Annex (Maybe (ImportableContentsChunkable Annex (ContentIdentifier, ByteSize))) listImportableContentsM ii dir = liftIO $ do - l <- dirContentsRecursive (fromRawFilePath dir) + l <- dirContentsRecursiveSkipping (const False) False (fromRawFilePath dir) l' <- mapM (go . toRawFilePath) l return $ Just $ ImportableContentsComplete $ ImportableContents (catMaybes l') [] diff --git a/doc/bugs/git-annex-import_imports_outside_of_directory.mdwn b/doc/bugs/git-annex-import_imports_outside_of_directory.mdwn index 7a6d18da8e..0adc7e253d 100644 --- a/doc/bugs/git-annex-import_imports_outside_of_directory.mdwn +++ b/doc/bugs/git-annex-import_imports_outside_of_directory.mdwn @@ -27,3 +27,5 @@ Linux ctchpcpx163.merck.com 3.10.0-1160.6.1.el7.x86_64 #1 SMP Tue Nov 17 13:59:1 That I use it enough to run into corner-case issues shows its continued usefulness :) [[!meta title="git-annex import follows symlinks"]] + +> [[fixed|done]] --[[Joey]] diff --git a/doc/git-annex-import.mdwn b/doc/git-annex-import.mdwn index 1a8a866c57..6ed9bec319 100644 --- a/doc/git-annex-import.mdwn +++ b/doc/git-annex-import.mdwn @@ -105,7 +105,8 @@ the tree of files on the remote, even when importing into a subdirectory. When run with a path, `git annex import` **moves** files from somewhere outside the git working copy, and adds them to the annex. In contrast to importing -from a special directory remote, imported files are **deleted from the given path**. +from a special directory remote, imported files are **deleted from the given +path**. This is a legacy interface. It is still supported, but please consider switching to importing from a directory special remote instead, using the @@ -132,6 +133,11 @@ Several options can be used to adjust handling of duplicate files, see `--duplicate`, `--deduplicate`, `--skip-duplicates`, `--clean-duplicates`, and `--reinject-duplicates` documentation below. +symbolic links in the directory being imported are skipped to avoid +accidentially importing things outside the directory that import was ran +on. The directory that import is run on can, however inself be a symbolic +link, and that symbolic link will be followed. + # OPTIONS FOR IMPORTING FROM A DIRECTORY * `--duplicate` -- 2.30.2