diff --git a/CHANGELOG.md b/CHANGELOG.md --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,19 @@ # ChangeLog for wai-app-file-cgi +## 3.3.0 + +- **Breaking**: `rawPathInfo` is percent-decoded in `fileApp` and + `cgiApp`, including `PATH_INFO` and `SCRIPT_NAME`. Callers that + decode it beforehand (e.g. mighttpd2 up to now) must pass the + original request; otherwise paths containing `%` are decoded + twice. +- **Breaking**: paths that do not match the source of a route are + rejected with 404 in all applications. A source with a trailing + slash also matches the path without it. +- **Breaking**: paths containing a segment starting with `.` (e.g. + `.git`), NUL, or encoded `..` are rejected with 400 in `fileApp` + and `cgiApp`. `/.well-known/` is allowed. + ## 3.2.3 - Security: request headers containing `_` are not passed to CGI diff --git a/Network/Wai/Application/Classic/CGI.hs b/Network/Wai/Application/Classic/CGI.hs --- a/Network/Wai/Application/Classic/CGI.hs +++ b/Network/Wai/Application/Classic/CGI.hs @@ -22,6 +22,7 @@ import Network.Wai.Conduit import Network.Wai.Application.Classic.Conduit import Network.Wai.Application.Classic.Field +import Network.Wai.Application.Classic.FileInfo (decodedPathInfo, isUnsafePath, matchRoute) import Network.Wai.Application.Classic.Header import Network.Wai.Application.Classic.Path import Network.Wai.Application.Classic.Types @@ -45,24 +46,28 @@ {-| Handle any methods for CGI. +Paths containing a segment starting with \'.\' are rejected with 400. + CGI processes are reaped by this library. The program to link this library should not ignore SIGCHLD. Otherwise, the PID of a finished CGI process may be reused by another process before it is terminated. -} cgiApp :: ClassicAppSpec -> CgiAppSpec -> CgiRoute -> Application cgiApp cspec spec cgii req respond - | hasDotDotSegment (rawPathInfo req) = + | isUnsafePath (rawPathInfo req) = respond $ responseLBS badRequest400 textPlainHeader "Bad Request\r\n" + | not (matchRoute (cgiSrc cgii) (decodedPathInfo req)) = notFound | otherwise = do exist <- doesFileExist prog if exist then E.bracket setup teardown cgi - else respond $ responseLBS notFound404 textPlainHeader "Not Found\r\n" + else notFound where + notFound = respond $ responseLBS notFound404 textPlainHeader "Not Found\r\n" cgiPath@(prog,_,_) = pathinfoToCGI (cgiSrc cgii) (cgiDst cgii) - (rawPathInfo req) + (decodedPathInfo req) (indexCgi spec) setup = execProcess cspec cgiPath req -- An unreaped process keeps its PID, so SIGTERM cannot hit diff --git a/Network/Wai/Application/Classic/File.hs b/Network/Wai/Application/Classic/File.hs --- a/Network/Wai/Application/Classic/File.hs +++ b/Network/Wai/Application/Classic/File.hs @@ -50,6 +50,9 @@ -- If 'rawPathInfo' does not end with \'/\' and a corresponding index file -- exist, redirection is specified in HTTP response. -- +-- Paths containing a segment starting with \'.\' (e.g. \".git\") +-- are rejected with 400, except \"\/.well-known\/\". +-- -- Directory contents are NOT automatically listed. To list directory -- contents, an index file must be created beforehand. -- @@ -58,7 +61,8 @@ fileApp :: ClassicAppSpec -> FileAppSpec -> FileRoute -> Application fileApp cspec spec filei req respond = do rspspec <- case method of - _ | hasDotDotSegment (rawPathInfo req) -> return badRequest + _ | isUnsafePath (rawPathInfo req) -> return badRequest + _ | not (matchRoute (fileSrc filei) (decodedPathInfo req)) -> return notFound Right GET -> processGET hinfo ishtml rfile Right HEAD -> processGET hinfo ishtml rfile _ -> return notAllowed diff --git a/Network/Wai/Application/Classic/FileInfo.hs b/Network/Wai/Application/Classic/FileInfo.hs --- a/Network/Wai/Application/Classic/FileInfo.hs +++ b/Network/Wai/Application/Classic/FileInfo.hs @@ -1,6 +1,9 @@ +{-# LANGUAGE OverloadedStrings #-} + module Network.Wai.Application.Classic.FileInfo where import qualified Data.ByteString as BS +import Network.HTTP.Types (urlDecode) import Network.Wai import Network.Wai.Application.Classic.Path import Network.Wai.Application.Classic.Types @@ -37,15 +40,65 @@ -- "/tmp/dst/" -- >>> pathinfoToFilePath (mkReq "/exact/match/more.html") (FileRoute "/exact/match" "/tmp/dst") -- "/tmp/dst/more.html" +-- >>> pathinfoToFilePath (mkReq "/no%5Fext%20ension") (FileRoute "/" "/srv/http/") +-- "/srv/http/no_ext ension" pathinfoToFilePath :: Request -> FileRoute -> Path pathinfoToFilePath req filei | BS.null path' = dst | otherwise = dst path' where - path = rawPathInfo req + path = decodedPathInfo req src = fileSrc filei dst = fileDst filei path' = path <\> src + +-- | 'rawPathInfo' with percent-encoding decoded. +decodedPathInfo :: Request -> Path +decodedPathInfo = urlDecode False . rawPathInfo + +-- | Whether the path is routed by the source of a route. +-- A source with a trailing slash also matches the path without it. +-- +-- >>> matchRoute "/static/" "/static/a.html" +-- True +-- >>> matchRoute "/static/" "/static" +-- True +-- >>> matchRoute "/static/" "/other/static/a.html" +-- False +-- >>> matchRoute "/exact/match" "/exact/match/more.html" +-- True +matchRoute :: Path -> Path -> Bool +matchRoute src path = + src `BS.isPrefixOf` path + || hasTrailingPathSeparator src && path == BS.init src + +-- | Whether the path, once decoded, contains NUL or a segment +-- starting with \'.\' (e.g. ".." and ".git"). +-- \"\/.well-known\/\" is allowed for ACME. +-- +-- >>> isUnsafePath "/foo/%2e%2e/bar" +-- True +-- >>> isUnsafePath "/foo%00.html" +-- True +-- >>> isUnsafePath "/foo/..bar" +-- True +-- >>> isUnsafePath "/%2egit/config" +-- True +-- >>> isUnsafePath "/foo.d/bar.html" +-- False +-- >>> isUnsafePath "/.well-known/acme-challenge/x" +-- False +-- >>> isUnsafePath "/.well-known/.git/config" +-- True +isUnsafePath :: Path -> Bool +isUnsafePath raw = BS.elem 0 path || any isHidden segs' + where + path = urlDecode False raw + segs = BS.splitWith (\w -> w == 47 || w == 92) path -- '/' and '\\' + segs' = case segs of + "" : ".well-known" : rest -> rest + _ -> segs + isHidden seg = "." `BS.isPrefixOf` seg -- | -- diff --git a/Network/Wai/Application/Classic/Redirect.hs b/Network/Wai/Application/Classic/Redirect.hs --- a/Network/Wai/Application/Classic/Redirect.hs +++ b/Network/Wai/Application/Classic/Redirect.hs @@ -8,12 +8,14 @@ import Network.HTTP.Types import Network.Wai import Network.Wai.Application.Classic.Field +import Network.Wai.Application.Classic.FileInfo (matchRoute) import Network.Wai.Application.Classic.Path import Network.Wai.Application.Classic.Types redirectApp :: ClassicAppSpec -> RedirectRoute -> Application -redirectApp _ route req respond = - respond $ responseLBS status hdr "" +redirectApp _ route req respond + | matchRoute src path = respond $ responseLBS status hdr "" + | otherwise = respond $ responseLBS notFound404 textPlainHeader "Not Found\r\n" where path = rawPathInfo req src = redirectSrc route diff --git a/Network/Wai/Application/Classic/RevProxy.hs b/Network/Wai/Application/Classic/RevProxy.hs --- a/Network/Wai/Application/Classic/RevProxy.hs +++ b/Network/Wai/Application/Classic/RevProxy.hs @@ -17,6 +17,7 @@ import Network.Wai.Application.Classic.Conduit import Network.Wai.Application.Classic.EventSource import Network.Wai.Application.Classic.Field +import Network.Wai.Application.Classic.FileInfo (matchRoute) import Network.Wai.Application.Classic.Path import Network.Wai.Application.Classic.Types import Network.Wai.Conduit @@ -28,10 +29,13 @@ -- | Relaying any requests as reverse proxy. revProxyApp :: ClassicAppSpec -> RevProxyAppSpec -> RevProxyRoute -> Application -revProxyApp cspec spec route req respond = do - responded <- newIORef False - H.withResponse httpClientRequest mgr (proxy responded) - `E.catch` badGateway responded +revProxyApp cspec spec route req respond + | not (matchRoute (revProxySrc route) (rawPathInfo req)) = + respond $ responseLBS notFound404 textPlainHeader "Not Found\r\n" + | otherwise = do + responded <- newIORef False + H.withResponse httpClientRequest mgr (proxy responded) + `E.catch` badGateway responded where proxy responded hrsp = do let status = H.responseStatus hrsp diff --git a/test/ClassicSpec.hs b/test/ClassicSpec.hs --- a/test/ClassicSpec.hs +++ b/test/ClassicSpec.hs @@ -18,6 +18,10 @@ let url = "http://127.0.0.1:2345/cgi-bin/" bdy <- rspBody <$> sendGET url bdy `shouldBe` "index.cgi\n" + it "decodes PATH_INFO" $ do + let url = "http://127.0.0.1:2345/cgi-bin/index.cgi/a%20b" + bdy <- rspBody <$> sendGET url + bdy `shouldBe` "PATH_INFO: /a b\n" it "accepts POST" $ do let url = "http://127.0.0.1:2345/cgi-bin/echo-env/pathinfo?query=foo" bdy <- rspBody <$> sendPOST url "foo bar.\nbaz!\n" @@ -61,6 +65,10 @@ let url = "http://127.0.0.1:2345/cgi-bin/broken" sc <- rspCode <$> sendPOST url "foo bar.\nbaz!\n" sc `shouldBe` (5, 0, 0) + it "returns 400 for hidden files" $ do + let url = "http://127.0.0.1:2345/cgi-bin/%2ehidden" + sc <- rspCode <$> sendGET url + sc `shouldBe` (4, 0, 0) it "returns 404 if the CGI does not exist" $ do let url = "http://127.0.0.1:2345/cgi-bin/nonexistent" sc <- rspCode <$> sendGET url @@ -89,6 +97,12 @@ bdy <- bracket_ (writeFile file "hello\n") (removeFile file) $ H.responseBody <$> H.httpLbs hreq' mgr bdy `shouldBe` "hello\n" + it "serves files with percent-encoded non-ASCII names" $ do + let url = "http://127.0.0.1:2345/%E6%97%A5%E6%9C%AC.txt" + file = "test/html/\26085\26412.txt" + bdy <- bracket_ (writeFile file "hello\n") (removeFile file) $ + rspBody <$> sendGET url + bdy `shouldBe` "hello\n" it "returns 404 if not exist" $ do let url = "http://127.0.0.1:2345/dummy" sc <- rspCode <$> sendGET url @@ -97,6 +111,23 @@ let url = "http://127.0.0.1:2345/../../../../../../etc/hosts" sc <- rspCode <$> sendGET url sc `shouldBe` (4, 0, 0) + it "returns 400 if the path contains encoded .." $ do + let url = "http://127.0.0.1:2345/%2e%2e/%2e%2e/%2e%2e/%2e%2e/etc/hosts" + sc <- rspCode <$> sendGET url + sc `shouldBe` (4, 0, 0) + it "returns 400 for hidden files even if encoded" $ do + sc1 <- rspCode <$> sendGET "http://127.0.0.1:2345/.git/config" + sc1 `shouldBe` (4, 0, 0) + sc2 <- rspCode <$> sendGET "http://127.0.0.1:2345/%2egit/config" + sc2 `shouldBe` (4, 0, 0) + it "does not reject /.well-known/" $ do + sc <- rspCode <$> sendGET "http://127.0.0.1:2345/.well-known/acme-challenge/x" + sc `shouldBe` (4, 0, 4) + it "decodes percent-encoded paths" $ do + let url = "http://127.0.0.1:2345/no%5Fextension" + bdy <- rspBody <$> sendGET url + ans <- readFileAscii "test/html/no_extension" + bdy `shouldBe` ans it "returns Japanese HTML if language is specified" $ do let url = "http://127.0.0.1:2345/ja/" bdy <- rspBody <$> sendGETwH url [Header HdrAcceptLanguage "ja, en;q=0.7"] @@ -156,6 +187,11 @@ rsp <- sendGET url lookupHeader HdrLocation (rspHeaders rsp) `shouldBe` Just "//127.0.0.1:2345/redirect/?foo=bar" + it "returns 404 if the path does not match the route" $ do + -- "/mismatch" has the same length as "/statics/". + let url = "http://127.0.0.1:2345/mismatchindex.html" + sc <- rspCode <$> sendGET url + sc `shouldBe` (4, 0, 4) it "returns index.html for /exact-route.html matching exactRoute" $ do let url = "http://127.0.0.1:2345/exact-route.html" bdy <- rspBody <$> sendGET url diff --git a/test/Spec.hs b/test/Spec.hs --- a/test/Spec.hs +++ b/test/Spec.hs @@ -50,6 +50,7 @@ | upstream = revProxyApp appSpec (RevProxyAppSpec mgr) upstreamRoute req | redirect = redirectApp appSpec redirectRoute req | cgi = cgiApp appSpec defaultCgiAppSpec cgiRoute req + | mismatch = fileApp appSpec defaultFileAppSpec mismatchRoute req | exact = fileApp appSpec defaultFileAppSpec exactRoute req | otherwise = fileApp appSpec defaultFileAppSpec fileRoute req where @@ -57,6 +58,7 @@ upstream = "/upstream/" `BS.isPrefixOf` rawPathInfo req redirect = "/redirect-app/" `BS.isPrefixOf` rawPathInfo req cgi = "/cgi-bin/" `BS.isPrefixOf` rawPathInfo req + mismatch = "/mismatch" `BS.isPrefixOf` rawPathInfo req exact = "/exact-route.html" `BS.isPrefixOf` rawPathInfo req appSpec = defaultClassicAppSpec{softwareName = "ClassicTester"} redirectRoute = @@ -73,6 +75,12 @@ FileRoute { fileSrc = "/exact-route.html" , fileDst = fromString (dir "test/html/index.html") + } + -- Dispatched although the source does not match. + mismatchRoute = + FileRoute + { fileSrc = "/statics/" + , fileDst = fromString (dir "test/html/") } fileRoute = FileRoute diff --git a/wai-app-file-cgi.cabal b/wai-app-file-cgi.cabal --- a/wai-app-file-cgi.cabal +++ b/wai-app-file-cgi.cabal @@ -1,6 +1,6 @@ cabal-version: 1.18 name: wai-app-file-cgi -version: 3.2.3 +version: 3.3.0 license: BSD3 license-file: LICENSE maintainer: Kazu Yamamoto