Fix SCP path traversal issue

This commit fixes an issue where a malicious SCP server could trick an
SCP client into writing files outside of a requested target directory.
Thanks go to Jaden Furtado for reporting this issue and providing
detailed reproduction steps.

Note: While this fix should prevent writing outside a target directory,
the SCP protocol is inherently insecure and may still be able to
overwrite arbitrary files within the target directory, even if they
don't match the requested source files to copy. To avoid this issue, it
is strongly recommended that SFTP be used in place of SCP.

Author: ronf

asyncssh/scp.py

@@ -136,6 +136,10 @@ def _parse_cd_args(args: bytes) -> Tuple[int, int, bytes]:
 
     try:
         permissions, size, name = args.split(None, 2)
+
+        if b'/' in name or b'\\' in name or name == b'..':
+            raise _scp_error(SFTPBadMessage, 'Invalid filename')
+
         return int(permissions, 8), int(size), name
     except ValueError:
         raise _scp_error(SFTPBadMessage,

tests/test_sftp.py

@@ -5737,6 +5737,9 @@ async def _handle_client(process):
 
                 if command.endswith('get_connection_lost'):
                     pass
+                elif command.endswith('get_invalid_filename_response'):
+                    await process.stdin.read(1)
+                    process.stdout.write('C0644 0 ../src\n')
                 elif command.endswith('get_dir_no_recurse'):
                     await process.stdin.read(1)
                     process.stdout.write('D0755 0 src\n')
@@ -5772,6 +5775,17 @@ async def _handle_client(process):
 
         return await cls.create_server(process_factory=_handle_client)
 
+    @asynctest
+    async def test_get_invalid_filename_response(self):
+        """Test receiving directory when recurse wasn't requested"""
+
+        try:
+            with self.assertRaises((SFTPBadMessage, SFTPConnectionLost)):
+                await scp((self._scp_server, 'get_invalid_filename_response'),
+                          'dst')
+        finally:
+            remove('dst')
+
     @asynctest
     async def test_get_directory_without_recurse(self):
         """Test receiving directory when recurse wasn't requested"""