Validate full file path in the static file handler #383
Conversation
|
The code looks good on first glance but I cannot accept it without tests. Please push test code after functional tests for static file handling (https://github.com/KeepSafe/aiohttp/blob/master/tests/test_web_functional.py#L263-347) |
|
It turns out that requiring tests is a great policy, because my first patch had an embarassing error in it: Interestingly enough, the current static file tests would not have caught that error, because they all use absolute paths in the request URLs -- for example, on this line I think that was not intended, so I changed those test and made them use short paths in the URLs (the tests still pass). |
| @@ -149,7 +149,7 @@ def __init__(self, name, prefix, directory, *, | |||
| 'GET', self.handle, name, expect_handler=expect_handler) | |||
| self._prefix = prefix | |||
| self._prefix_len = len(self._prefix) | |||
| self._directory = directory | |||
| self._directory = os.path.abspath(directory) + os.sep | |||
There was a problem hiding this comment.
Please add check for directory like https://github.com/KeepSafe/aiohttp/blob/master/aiohttp/web_urldispatcher.py#L397
I guess to change exception from AssertionError to ValueError.
Agree. |
|
Like this? |
|
Perfect! But you forgot to add unttest for that... |
|
I'm not really sure where to put it though. Is this OK? |
Validate full file path in the static file handler
|
Yes |
|
Thanks! |
Hopefully prevent directory traversal attacks; as discussed in issue #380.