diff options
| -rw-r--r-- | Misc/NEWS.d/next/Security/2021-03-13-03-48-14.bpo-43285.g-Hah3.rst | 8 | ||||
| -rw-r--r-- | lib-python/2.7/ftplib.py | 9 | ||||
| -rw-r--r-- | lib-python/2.7/test/test_ftplib.py | 29 |
3 files changed, 43 insertions, 3 deletions
diff --git a/Misc/NEWS.d/next/Security/2021-03-13-03-48-14.bpo-43285.g-Hah3.rst b/Misc/NEWS.d/next/Security/2021-03-13-03-48-14.bpo-43285.g-Hah3.rst new file mode 100644 index 0000000000..8312b7e885 --- /dev/null +++ b/Misc/NEWS.d/next/Security/2021-03-13-03-48-14.bpo-43285.g-Hah3.rst @@ -0,0 +1,8 @@ +:mod:`ftplib` no longer trusts the IP address value returned from the server +in response to the PASV command by default. This prevents a malicious FTP +server from using the response to probe IPv4 address and port combinations +on the client network. + +Code that requires the former vulnerable behavior may set a +``trust_server_pasv_ipv4_address`` attribute on their +:class:`ftplib.FTP` instances to ``True`` to re-enable it. diff --git a/lib-python/2.7/ftplib.py b/lib-python/2.7/ftplib.py index 6644554792..0550f0ab9f 100644 --- a/lib-python/2.7/ftplib.py +++ b/lib-python/2.7/ftplib.py @@ -108,6 +108,8 @@ class FTP: file = None welcome = None passiveserver = 1 + # Disables https://bugs.python.org/issue43285 security if set to True. + trust_server_pasv_ipv4_address = False # Initialization method (called by class instantiation). # Initialize host to localhost, port to standard ftp port @@ -310,8 +312,13 @@ class FTP: return sock def makepasv(self): + """Internal: Does the PASV or EPSV handshake -> (address, port)""" if self.af == socket.AF_INET: - host, port = parse227(self.sendcmd('PASV')) + untrusted_host, port = parse227(self.sendcmd('PASV')) + if self.trust_server_pasv_ipv4_address: + host = untrusted_host + else: + host = self.sock.getpeername()[0] else: host, port = parse229(self.sendcmd('EPSV'), self.sock.getpeername()) return host, port diff --git a/lib-python/2.7/test/test_ftplib.py b/lib-python/2.7/test/test_ftplib.py index 416ee46439..bc15ba8aab 100644 --- a/lib-python/2.7/test/test_ftplib.py +++ b/lib-python/2.7/test/test_ftplib.py @@ -67,6 +67,10 @@ class DummyFTPHandler(asynchat.async_chat): self.rest = None self.next_retr_data = RETR_DATA self.push('220 welcome') + # We use this as the string IPv4 address to direct the client + # to in response to a PASV command. To test security behavior. + # https://bugs.python.org/issue43285/. + self.fake_pasv_server_ip = '252.253.254.255' def collect_incoming_data(self, data): self.in_buffer.append(data) @@ -109,8 +113,9 @@ class DummyFTPHandler(asynchat.async_chat): sock.bind((self.socket.getsockname()[0], 0)) sock.listen(5) sock.settimeout(10) - ip, port = sock.getsockname()[:2] - ip = ip.replace('.', ',') + port = sock.getsockname()[1] + ip = self.fake_pasv_server_ip + ip = ip.replace('.', ','); p1 = port // 256; p2 = port % 256 p1, p2 = divmod(port, 256) self.push('227 entering passive mode (%s,%d,%d)' %(ip, p1, p2)) conn, addr = sock.accept() @@ -583,6 +588,26 @@ class TestFTPClass(TestCase): # IPv4 is in use, just make sure send_epsv has not been used self.assertEqual(self.server.handler_instance.last_received_cmd, 'pasv') + def test_makepasv_issue43285_security_disabled(self): + """Test the opt-in to the old vulnerable behavior.""" + self.client.trust_server_pasv_ipv4_address = True + bad_host, port = self.client.makepasv() + self.assertEqual( + bad_host, self.server.handler_instance.fake_pasv_server_ip) + # Opening and closing a connection keeps the dummy server happy + # instead of timing out on accept. + socket.create_connection((self.client.sock.getpeername()[0], port), + timeout=TIMEOUT).close() + + def test_makepasv_issue43285_security_enabled_default(self): + self.assertFalse(self.client.trust_server_pasv_ipv4_address) + trusted_host, port = self.client.makepasv() + self.assertNotEqual( + trusted_host, self.server.handler_instance.fake_pasv_server_ip) + # Opening and closing a connection keeps the dummy server happy + # instead of timing out on accept. + socket.create_connection((trusted_host, port), timeout=TIMEOUT).close() + def test_line_too_long(self): self.assertRaises(ftplib.Error, self.client.sendcmd, 'x' * self.client.maxline * 2) |