diff --git a/Lib/ftplib.py b/Lib/ftplib.py index 1f760ed1ce0bf09e7355f54a90393f6c4ee8cf57..7c5a50715f6dc691443ba89bfef07bbbea7fdc1d 100644 --- a/Lib/ftplib.py +++ b/Lib/ftplib.py @@ -102,7 +102,9 @@ class FTP: sock = None file = None welcome = None - passiveserver = 1 + passiveserver = True + # Disables https://bugs.python.org/issue43285 security if set to True. + trust_server_pasv_ipv4_address = False def __init__(self, host='', user='', passwd='', acct='', timeout=_GLOBAL_DEFAULT_TIMEOUT, source_address=None, *, @@ -320,8 +322,13 @@ def makeport(self): 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/test/test_ftplib.py b/Lib/test/test_ftplib.py index e424076d7d317c6b40a6a5aaf2d8f4b179a6f859..b76287bfe1c2741fa09834fee6be1a888f6ec6b3 100644 --- a/Lib/test/test_ftplib.py +++ b/Lib/test/test_ftplib.py @@ -101,6 +101,10 @@ def __init__(self, conn, encoding=DEFAULT_ENCODING): self.next_retr_data = RETR_DATA self.push('220 welcome') self.encoding = encoding + # 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) @@ -141,7 +145,8 @@ def cmd_port(self, arg): def cmd_pasv(self, arg): with socket.create_server((self.socket.getsockname()[0], 0)) as sock: sock.settimeout(TIMEOUT) - ip, port = sock.getsockname()[:2] + port = sock.getsockname()[1] + ip = self.fake_pasv_server_ip ip = ip.replace('.', ','); p1 = port / 256; p2 = port % 256 self.push('227 entering passive mode (%s,%d,%d)' %(ip, p1, p2)) conn, addr = sock.accept() @@ -705,6 +710,26 @@ def test_makepasv(self): # 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_with_statement(self): self.client.quit()