FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-43285 Make ftplib not trust the PASV response. by gpshead · Pull Request #24838 · python/cpython · GitHub

/ cpython Public

bpo-43285 Make ftplib not trust the PASV response. #24838

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gpshead merged 2 commits into from
Mar 15, 2021
Conversation 8 Commits 2 Checks 0 Files changed
Merged

bpo-43285 Make ftplib not trust the PASV response. #24838

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4b397c1
bpo-43285 Make ftplib not trust the PASV response.
gpshead Mar 13, 2021
6003e2f
Rename the legacy behavior attribute and mention it in NEWS.
gpshead Mar 13, 2021
File filter

Filter by extension

Filter by extension .py  (2) .rst  (1) All 2 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
11 changes: 9 additions & 2 deletions Lib/ftplib.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -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, *,
Expand Down Expand Up @@ -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
Expand Down
27 changes: 26 additions & 1 deletion Lib/test/test_ftplib.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,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)
Expand Down Expand Up @@ -143,7 +147,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()
Expand Down Expand Up @@ -707,6 +712,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()

Expand Down
8 changes: 8 additions & 0 deletions Misc/NEWS.d/next/Security/2021-03-13-03-48-14.bpo-43285.g-Hah3.rst
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -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.
Back | FazBrowse Home | New Git URL