StaticRoute is vulnerable to directory traversal attacks #380
Description
Hi. There is a problem in the way StaticRoute class tries to prevent directory traversal attacks.
Currently it obtains the full file path by gluing the base directory with the requested path via os.path.join, and then checks if ".." is in the resulting path -- if there is, the path is rejected, otherwise, it is accepted. Here's the relevant code (taken from here):
@asyncio.coroutine
def handle(self, request):
resp = StreamResponse()
filename = request.match_info['filename']
filepath = os.path.join(self._directory, filename)
if '..' in filename:
raise HTTPNotFound()
if not os.path.exists(filepath) or not os.path.isfile(filepath):
raise HTTPNotFound()
...Unfortunately, os.path.join has a quirk: if it determines that it's second argument is an absolute path, it will ignore the first argument completely, and only return the second one, so if filename above is an absolute path, filepath will just be this path. Therefore, the code above does not protect against directory traversal attacks.
Here's a simple proof of concept:
import asyncio
from aiohttp import web
@asyncio.coroutine
def init(loop, address, service):
app = web.Application(loop=loop)
app.router.add_static("/data/", "data")
server = yield from loop.create_server(app.make_handler(), address, service)
return server
loop = asyncio.get_event_loop()
loop.run_until_complete(init(loop, "127.0.0.1", 8000))
loop.run_forever()Run this program and direct your browser to one of these addresses (depending on which OS you're using):
http://127.0.0.1:8000/data//etc/passwd
http://127.0.0.1:8000/data/c:/config.sys
... and you'll obtain data which you should not be able to see.
OK, so there doesn't seem to be a single widely accepted way to sanitize relative paths; for example Django does this whole dance, while Bottle only does this. Both approaches seem to fix the current problem, so I ask you to adopt something of this sort.