Repository: aio-libs/aioftp

Issue:
Directory traversal / base path escape possible on Windows
The server allows escaping the user `base_path` on Windows using absolute path (segments).
This is due to [`get_paths`](https://github.com/aio-libs/aioftp/blob/81d4bc018261268bf2ea88f42e05f130ab4c9229/aioftp/server.py#L991):
While the virtual path handling is perfectly fine in itself (it represents a POSIX path, so all is good there), the joining between `base_path` and `resolved_virtual_path` is not and quickly falls apart when Windows absolute paths are involved.
In my opinion this presets a serious security issue when developers naively rely on the `base_path` setting to not allow users to traverse outside of it.

Consider the following:
```python
base_path = pathlib.Path("C:\\locked_path")  # base_path is of the type Path (so platform dependent path)
```
Now see what happens when the input virtual path contains Windows absolute path segments:
```python
resolved_virtual_path = pathlib.PurePosixPath("/foo/C:\\Windows/bla")
real_path = base_path / resolved_virtual_path.relative_to("/")
>>> PureWindowsPath('C:/Windows/bla')
```
You have now broken out.
This is due to two reasons:
1. While joining, `pathlib` joins each path segment to the list of first path object with the following rule:
    > When several absolute paths are given, the last is taken as an anchor (mimicking os.path.join()’s behaviour)
    This is also the reason the `relative_to("/")` does not help here.
2. When joining, `pathlib` joins according to the rules of the first path object, so in this case it pulls out all path `parts` from the `PurePosixPath` and joins them to the `PureWindowsPath` by the rules of `PureWindowsPath`, meaning the Windows absolute anchor in the `PurePosixPath` is interpreted as on Windows.

There are a myriad of ways to _almost_ fix it inside the virtual path, but they always have special cases that break them and I do not consider them safe.
In my opinion, the best way to prevent any directory traversal using `pathlib` is by going by the resulting absolute path.
1. Join the `resolved_virtual_path` to the `base_path` as is already done, but using the anchor given in the path to be relative to since users can pass Windows anchors too:
   ```
   real_path = base_path / resolved_virtual_path.relative_to(resolved_virtual_path.anchor)
   ```
2. Use `relative_to` to make sure the joined `real_path` is still relative to / inside the `base_path` and give us the fully resolved (all absolute anchors removed) subdirectory/virtual path at the same time:
   ```
   resolved_virtual_path = pathlib.PurePosixPath("/") / real_path.relative_to(base_path)
   ```
3. If the previous call raises (with our example from above: `ValueError: 'C:\\Windows\\bla' is not in the subpath of 'C:\\locked_path' OR one path is relative and the other is absolute.`) we know that a traversal was attempted and can either return 
    1. An access denied error (not very fitting in the current design, would need to either convert to using exceptions or add lots of boilerplate)
    2. Just return something like `base_path, pathlib.PurePosixPath("/")` to throw the user back to the home directory.

I will submit a pull request shortly.

Discussion:

Patch:
diff --git a/aioftp/server.py b/aioftp/server.py
index 4af6399..f0a92c3 100644
--- a/aioftp/server.py
+++ b/aioftp/server.py
@@ -988,7 +988,8 @@ class Server:
             if tasks_to_wait:
                 await asyncio.wait(tasks_to_wait)
 
-    def get_paths(self, connection, path):
+    @staticmethod
+    def get_paths(connection, path):
         """
         Return *real* and *virtual* paths, resolves ".." with "up" action.
         *Real* path is path for path_io, when *virtual* deals with
@@ -1014,6 +1015,12 @@ class Server:
                 resolved_virtual_path /= part
         base_path = connection.user.base_path
         real_path = base_path / resolved_virtual_path.relative_to("/")
+        # replace with `is_relative_to` check after 3.9+ requirements lands
+        try:
+            real_path.relative_to(base_path)
+        except ValueError:
+            real_path = base_path
+            resolved_virtual_path = pathlib.PurePosixPath("/")
         return real_path, resolved_virtual_path
 
     async def greeting(self, connection, rest):


Test patch:
diff --git a/tests/test_simple_functions.py b/tests/test_simple_functions.py
index e46448b..5fbc312 100644
--- a/tests/test_simple_functions.py
+++ b/tests/test_simple_functions.py
@@ -230,3 +230,17 @@ def test_server_mtime_build():
     b = aioftp.Server.build_list_mtime
     assert b(now, now) == "Jan  1 00:00"
     assert b(past, now) == "Jan  1  2001"
+
+
+def test_get_paths_windows_traverse():
+    base_path = pathlib.PureWindowsPath("C:\\ftp")
+    user = aioftp.User()
+    user.base_path = base_path
+    connection = aioftp.Connection(current_directory=base_path, user=user)
+    virtual_path = pathlib.PurePosixPath("/foo/C:\\windows")
+    real_path, resolved_virtual_path = aioftp.Server.get_paths(
+        connection,
+        virtual_path,
+    )
+    assert real_path == base_path
+    assert resolved_virtual_path == pathlib.PurePosixPath("/")