diff --git a/Annex/UntrustedFilePath.hs b/Annex/UntrustedFilePath.hs index c843e8042e..2ec37842b6 100644 --- a/Annex/UntrustedFilePath.hs +++ b/Annex/UntrustedFilePath.hs @@ -25,9 +25,14 @@ import System.FilePath - so no dotfiles that might control a program are inadvertently created, - and to avoid filenames being treated as options to commands the user - might run. + - + - Also there's an off chance the string might be empty, so to avoid + - needing to handle such an invalid filepath, return a dummy "file" in + - that case. -} sanitizeFilePath :: String -> FilePath -sanitizeFilePath = leading . map sanitize +sanitizeFilePath [] = "file" +sanitizeFilePath f = leading (map sanitize f) where sanitize c | c == '.' || c == '-' = c diff --git a/Command/AddUrl.hs b/Command/AddUrl.hs index 73746e8c1a..e25a420b9b 100644 --- a/Command/AddUrl.hs +++ b/Command/AddUrl.hs @@ -136,13 +136,13 @@ checkUrl addunlockedmatcher r o u = do go _ (Left e) = void $ commandAction $ startingAddUrl u o $ do warning (show e) next $ return False - go deffile (Right (UrlContents sz mf)) = do - let f = adjustFile o (fromMaybe (maybe deffile fromSafeFilePath mf) (fileOption (downloadOptions o))) + go deffile (Right (UrlContents sz f)) = do + let f = adjustFile o (fromMaybe (maybe deffile sanitizeFilePath mf) (fileOption (downloadOptions o))) void $ commandAction $ startRemote addunlockedmatcher r o f u sz go deffile (Right (UrlMulti l)) = case fileOption (downloadOptions o) of Nothing -> forM_ l $ \(u', sz, f) -> do - let f' = adjustFile o (deffile fromSafeFilePath f) + let f' = adjustFile o (deffile sanitizeFilePath f) void $ commandAction $ startRemote addunlockedmatcher r o f' u' sz Just f -> case l of [] -> noop diff --git a/Command/ImportFeed.hs b/Command/ImportFeed.hs index be03ccb793..6eb679ea71 100644 --- a/Command/ImportFeed.hs +++ b/Command/ImportFeed.hs @@ -190,7 +190,7 @@ performDownload addunlockedmatcher opts cache todownload = case location todownl downloadRemoteFile addunlockedmatcher r (downloadOptions opts) url f sz Right (UrlMulti l) -> do kl <- forM l $ \(url', sz, subf) -> - downloadRemoteFile addunlockedmatcher r (downloadOptions opts) url' (f fromSafeFilePath subf) sz + downloadRemoteFile addunlockedmatcher r (downloadOptions opts) url' (f sanitizeFilePath subf) sz return $ if all isJust kl then catMaybes kl else [] diff --git a/Remote/BitTorrent.hs b/Remote/BitTorrent.hs index 245081cf2e..e0191d04e3 100644 --- a/Remote/BitTorrent.hs +++ b/Remote/BitTorrent.hs @@ -401,8 +401,8 @@ torrentContents :: URLString -> Annex UrlContents torrentContents u = convert <$> (liftIO . torrentFileSizes =<< tmpTorrentFile u) where - convert [(fn, sz)] = UrlContents (Just sz) (Just (mkSafeFilePath fn)) + convert [(fn, sz)] = UrlContents (Just sz) (Just fn) convert l = UrlMulti $ map mkmulti (zip l [1..]) mkmulti ((fn, sz), n) = - (torrentUrlWithNum u n, Just sz, mkSafeFilePath $ joinPath $ drop 1 $ splitPath fn) + (torrentUrlWithNum u n, Just sz, joinPath $ drop 1 $ splitPath fn) diff --git a/Remote/External.hs b/Remote/External.hs index 72f1a2c52c..27f6f84047 100644 --- a/Remote/External.hs +++ b/Remote/External.hs @@ -769,14 +769,14 @@ checkUrlM :: External -> URLString -> Annex UrlContents checkUrlM external url = handleRequest external (CHECKURL url) Nothing $ \req -> case req of CHECKURL_CONTENTS sz f -> result $ UrlContents sz $ - if null f then Nothing else Just $ mkSafeFilePath f + if null f then Nothing else Just f CHECKURL_MULTI l -> result $ UrlMulti $ map mkmulti l CHECKURL_FAILURE errmsg -> Just $ giveup $ respErrorMessage "CHECKURL" errmsg UNSUPPORTED_REQUEST -> giveup "CHECKURL not implemented by external special remote" _ -> Nothing where - mkmulti (u, s, f) = (u, s, mkSafeFilePath f) + mkmulti (u, s, f) = (u, s, f) retrieveUrl :: Retriever retrieveUrl = fileRetriever $ \f k p -> do diff --git a/Types/UrlContents.hs b/Types/UrlContents.hs index 7cbfe37f88..c2d2ca86ad 100644 --- a/Types/UrlContents.hs +++ b/Types/UrlContents.hs @@ -7,35 +7,14 @@ module Types.UrlContents ( UrlContents(..), - SafeFilePath, - mkSafeFilePath, - fromSafeFilePath ) where import Utility.Url -import Annex.UntrustedFilePath - -import System.FilePath data UrlContents -- An URL contains a file, whose size may be known. -- There might be a nicer filename to use. - = UrlContents (Maybe Integer) (Maybe SafeFilePath) + = UrlContents (Maybe Integer) (Maybe FilePath) -- Sometimes an URL points to multiple files, each accessible -- by their own URL. - | UrlMulti [(URLString, Maybe Integer, SafeFilePath)] - --- This is a FilePath, from an untrusted source, --- sanitized so it doesn't contain any directory traversal tricks --- and is always relative. It can still contain subdirectories. --- Any unusual characters are also filtered out. -newtype SafeFilePath = SafeFilePath FilePath - deriving (Show) - -mkSafeFilePath :: FilePath -> SafeFilePath -mkSafeFilePath p = SafeFilePath $ if null p' then "file" else p' - where - p' = joinPath $ map sanitizeFilePath $ splitDirectories p - -fromSafeFilePath :: SafeFilePath -> FilePath -fromSafeFilePath (SafeFilePath p) = p + | UrlMulti [(URLString, Maybe Integer, FilePath)]