Odd behaviour with static policy using addBase

[hdgarrood]

I wanted to serve static files from src/static, under /static (so GET /static/some-file would look for src/static/some-file), so I did this:

let p = noDots >-> hasPrefix "/static" >-> addBase "src"
staticPolicy p

This didn't quite do what I expected:

tryPolicy p "/static/some-file" == Just "/static/some-file"

I think it's because of </> in System.FilePath; it seems it just returns the second argument if it starts with a slash:

"src" </> "/static/some-file" == "/static/some-file"

However curl and firefox both seem to always add this leading slash in the request URI.

Should addBase use ++ rather than FP.</>?

[hdgarrood]

Actually, come to think of it, maybe it would work better to redo policies so that they expect the request URI as a [Text] rather than String. That way, it removes the need for addSlash (and any other gotchas involving leading/trailing slashes), and it makes it easy to pattern match:

findStaticFile :: [Text] -> Maybe [Text]
findStaticFile ("static":xs) = Just $ "src":xs
findStaticFile _             = Nothing

myPolicy :: Policy
myPolicy = noDots >-> policy findStaticFile

Then wai-middleware-static could do T.unpack . T.intercalate "/" at the end to get the filepath.

[daapp]

addBase is insecure, if you do:
curl -i http://scottystarter.herokuapp.com///etc/passwd
you will see someones passwd file

My version of this code is:

import System.FilePath (isAbsolute)

addSecureBase :: String -> Policy
addSecureBase b = policy (\path -> if isAbsolute path then Nothing else Just (b </> path))

But addBase may be usefull for some cases.

[hdgarrood]

I worked out why my code wasn't working; you just need to not include the leading slash in the addBase:

addBase "static"

@daapp raises a good point. I've just reproduced it locally:

scotty 3000 $ middleware (staticPolicy $ addBase "src")

Then curl localhost:3000//etc/passwd will print out the contents of /etc/passwd.

[co-dan]

@daapp wow, this is really bad. I think your addSecureBase should be a default implementation.

[co-dan]

@xich, what do you think about that?

[daapp]

@co-dan I also recommend to add noDots to default implementation too. But also leave the way to ignore all these security locks.

[xich]

Ugh, thanks for pointing this out... I'll work on fixing this, though patches are welcome! ;-)

[xich]

Merged and released as wai-middleware-static 0.5.0.0. Thanks for the patch @nhibberd

This should fix the absolute path issue (to /etc/passwd for example), but I'm leaving this issue open for now because I think @hdgarrood was right in that policies should work over [Text] (or [ByteString]) rather than String, to keep separators abstract.

Also, if anyone finds other gaping security problems, with wai-middleware-static or scotty itself, please let me know!