diff --git a/CHANGELOG.md b/CHANGELOG.md --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,27 @@ # Revision history for spacecookie +## 1.0.0.3 + +2025-05-03 + +**Security fix**: +Resolve `sanitizePath` not eliminating `..` from paths. This affects users +of `sanitizePath` and `sanitizePathIfNotUrl` from `Network.Gopher.Util`. + +This issue only affects the spacecookie library, not the spacecookie server +daemon since a separate check would prevent it from handling such malicious +requests (which delayed the discovery of this bug). It is probably wise to +upgrade either way. + +Note that gophermap parsing behavior is unchanged, i.e. it just `normalise`s +paths, even though `makeGophermapFilePath` used to call `sanitizePath` in +previous versions. This is due to the assumption that gophermaps come from a +trusted source and/or paths produced from gophermap parsing aren't used to +access files directly, i.e. those paths are only served to clients (whose later +requests are subject to selector sanitization) as selectors in menus. If those +assumptions don't hold for your code, you will need to further sanitize the +paths returned from `gophermapToDirectoryResponse`. + ## 1.0.0.2 2022-10-03 @@ -25,7 +47,7 @@ added, but old configuration stays compatible. However some gophermap files may need adjusting, especially if they contain absolute paths not starting with a slash. -* For library users there are multiple braking changes to the core API +* For library users there are multiple breaking changes to the core API that probably need adjusting in downstream usage as well as some changes to behavior. diff --git a/server/Network/Spacecookie/FileType.hs b/server/Network/Spacecookie/FileType.hs --- a/server/Network/Spacecookie/FileType.hs +++ b/server/Network/Spacecookie/FileType.hs @@ -4,6 +4,7 @@ , gopherFileType -- exposed for tests , lookupSuffix + , checkNoDotFiles ) where import Control.Applicative ((<|>)) diff --git a/spacecookie.cabal b/spacecookie.cabal --- a/spacecookie.cabal +++ b/spacecookie.cabal @@ -1,6 +1,6 @@ cabal-version: 3.0 name: spacecookie -version: 1.0.0.2 +version: 1.0.0.3 synopsis: Gopher server library and daemon description: Simple gopher library that allows writing custom gopher applications. Also includes a fully-featured gopher server @@ -94,6 +94,7 @@ other-modules: Test.FileTypeDetection , Test.Gophermap , Test.Integration + , Test.Sanitization , Network.Spacecookie.FileType build-depends: tasty >=1.2 , tasty-hunit >=0.10 @@ -104,8 +105,8 @@ source-repository head type: git - location: git://github.com/sternenseemann/spacecookie.git + location: https://github.com/sternenseemann/spacecookie.git source-repository head type: git - location: git://code.sterni.lv/spacecookie + location: https://code.sterni.lv/spacecookie diff --git a/src/Network/Gopher/Util.hs b/src/Network/Gopher/Util.hs --- a/src/Network/Gopher/Util.hs +++ b/src/Network/Gopher/Util.hs @@ -27,7 +27,7 @@ import Data.Char (ord, chr, toLower) import qualified Data.String.UTF8 as U import Data.Word (Word8 ()) -import System.FilePath.Posix.ByteString (RawFilePath, normalise, joinPath, splitPath) +import System.FilePath.Posix.ByteString (RawFilePath, normalise, joinPath, splitPath, equalFilePath) import System.Posix.User -- | 'chr' a 'Word8' @@ -67,8 +67,11 @@ -- | Normalise a path and prevent . sanitizePath :: RawFilePath -> RawFilePath -sanitizePath = joinPath - . filter (\p -> p /= ".." && p /= ".") +sanitizePath = + -- To retain prior behavior @"."@ after normalisation is mapped to @""@ + (\p -> if p == "." then "" else p) + . joinPath + . filter (\p -> not (equalFilePath p "..")) . splitPath . normalise -- | Use 'sanitizePath' except if the path starts with @URL:@ diff --git a/src/Network/Gopher/Util/Gophermap.hs b/src/Network/Gopher/Util/Gophermap.hs --- a/src/Network/Gopher/Util/Gophermap.hs +++ b/src/Network/Gopher/Util/Gophermap.hs @@ -40,7 +40,7 @@ import Data.Maybe (fromMaybe) import qualified Data.String.UTF8 as U import Data.Word (Word8 ()) -import System.FilePath.Posix.ByteString (RawFilePath, ()) +import System.FilePath.Posix.ByteString (RawFilePath, (), isAbsolute, normalise) -- | Given a directory and a Gophermap contained within it, -- return the corresponding gopher menu response. @@ -54,6 +54,8 @@ where realPath p = case p of GophermapAbsolute p' -> p' + -- TODO: `..` should be resolved textually for linking to the + -- parent directory (if possible) GophermapRelative p' -> dir p' GophermapUrl u -> u @@ -77,10 +79,12 @@ -- considered as an external URL and left as-is. -- * everything else is considered a relative path makeGophermapFilePath :: ByteString -> GophermapFilePath -makeGophermapFilePath b = - fromMaybe (GophermapRelative $ sanitizePath b) - $ boolToMaybe ("URL:" `isPrefixOf` b) (GophermapUrl b) - <|> boolToMaybe ("/" `isPrefixOf` b) (GophermapAbsolute $ sanitizePath b) +makeGophermapFilePath b + | "URL:" `isPrefixOf` b = GophermapUrl b + | isAbsolute b = GophermapAbsolute normalisedPath + | otherwise = GophermapRelative normalisedPath + where + normalisedPath = normalise b -- | A gophermap entry makes all values of a gopher menu item optional except for file type and description. When converting to a 'GopherMenuItem', appropriate default values are used. data GophermapEntry = GophermapEntry diff --git a/test/EntryPoint.hs b/test/EntryPoint.hs --- a/test/EntryPoint.hs +++ b/test/EntryPoint.hs @@ -4,6 +4,8 @@ -- library tests import Test.Gophermap +-- library-ish +import Test.Sanitization -- server executable tests import Test.FileTypeDetection @@ -15,6 +17,7 @@ tests :: TestTree tests = testGroup "tests" [ gophermapTests + , sanitizationTests , fileTypeDetectionTests , integrationTests ] diff --git a/test/Test/Sanitization.hs b/test/Test/Sanitization.hs new file mode 100644 --- /dev/null +++ b/test/Test/Sanitization.hs @@ -0,0 +1,64 @@ +{-# LANGUAGE OverloadedStrings #-} +module Test.Sanitization (sanitizationTests) where + +import Network.Gopher.Util (uEncode, sanitizePath) +import Network.Spacecookie.FileType (checkNoDotFiles, PathError (..)) + +import Control.Monad (forM_) +import Test.Tasty +import Test.Tasty.HUnit + +sanitizationTests :: TestTree +sanitizationTests = testGroup "Sanitization of user input" + [ pathSanitization + , dotFileDetectionTest + ] + +pathSanitization :: TestTree +pathSanitization = testCase "sanitizePath behavior" $ do + let assertSanitize e p = assertEqual p e $ sanitizePath (uEncode p) + assertSanitize "/root" "/root" + assertSanitize "/home/alice/.emacs.d/init.el" "/home/alice/.emacs.d/init.el" + + assertSanitize "root" "./root" + assertSanitize"/tools/magrathea" "//tools/magrathea" + assertSanitize "/home/bob/Documents/important.txt" "/home/bob//Documents/important.txt" + + assertSanitize "foo/bar/baz.txt" "./foo/bar/./baz.txt" + assertSanitize "/var/www/index..html" "/var/www/.///index..html" + assertSanitize "./" "./." + assertSanitize "/" "/." + assertSanitize "home/eve/" "./home/./././eve////./." + + assertSanitize "/home/bob/alice/private.txt" "/home/bob/../alice/private.txt" + +dotFileDetectionTest :: TestTree +dotFileDetectionTest = testCase "spacecookie server detects dot files in paths" $ do + let assertDot p hasDot = forM_ + [ (p, uEncode p) + , (p ++ " (sanitized)", sanitizePath (uEncode p)) + ] + $ \(title, path) -> assertEqual title + (if hasDot then Left PathIsNotAllowed else Right ()) + $ checkNoDotFiles path + + assertDot "./normal/relative/path" False + assertDot "." False + assertDot "/some/absolute/path" False + assertDot "file.txt" False + assertDot "/foo.html" False + assertDot "./tmp/scratch.txt" False + + assertDot ".emacs.d/init.el" True + assertDot ".gophermap" True + assertDot "/home/bob/.vimrc" True + assertDot "/home/alice/.config/foot" True + assertDot "./nixpkgs/.git/config" True + + -- only fail prior to sanitization + forM_ + [ "./.", "relative/./path", "dir/../traversal/../attack", "../../../actual/traversal" ] + $ \p -> do + let p' = uEncode p + assertEqual p (Left PathIsNotAllowed) $ checkNoDotFiles p' + assertEqual p (Right ()) $ checkNoDotFiles (sanitizePath p')