Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion Lib/ftplib.py
Original file line number Diff line number Diff line change
Expand Up @@ -883,7 +883,16 @@ def ftpcp(source, sourcename, target, targetname = '', type = 'I'):
type = 'TYPE ' + type
source.voidcmd(type)
target.voidcmd(type)
sourcehost, sourceport = parse227(source.sendcmd('PASV'))
# Don't trust the IPv4 address the source server advertises in its PASV
# reply: a malicious source could otherwise point the target's data
# connection at an arbitrary host (SSRF). A caller that needs the old
# behavior can set trust_server_pasv_ipv4_address on the source FTP
# object. See FTP.makepasv(), which applies the same rule.
untrusted_host, sourceport = parse227(source.sendcmd('PASV'))
if source.trust_server_pasv_ipv4_address:
sourcehost = untrusted_host
else:
sourcehost = source.sock.getpeername()[0]
target.sendport(sourcehost, sourceport)
# RFC 959: the user must "listen" [...] BEFORE sending the
# transfer request.
Expand Down
54 changes: 54 additions & 0 deletions Lib/test/test_ftplib.py
Original file line number Diff line number Diff line change
Expand Up @@ -1145,6 +1145,60 @@ def testTimeoutDirectAccess(self):
ftp.close()


class TestFtpcpSecurity(TestCase):
"""ftpcp() must not trust the host a source server advertises in PASV.

A malicious source server can otherwise redirect the target server's
data connection to an arbitrary host:port (SSRF), so ftpcp() uses the
source server's actual peer address instead, the same as FTP.makepasv().
"""

class _FakeSock:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need those fake classes or can't we just use Mock() objects instead? Just asking.

def __init__(self, peer_host):
self._peer = (peer_host, 21)
def getpeername(self):
return self._peer

class _FakeSource:
trust_server_pasv_ipv4_address = False
def __init__(self, advertised_host, real_host):
self.sock = TestFtpcpSecurity._FakeSock(real_host)
self._advertised = advertised_host.replace('.', ',')
def voidcmd(self, cmd):
pass
def sendcmd(self, cmd):
if cmd == 'PASV':
return '227 Entering Passive Mode (%s,1,2).' % self._advertised
return '150 ok'
def voidresp(self):
pass

class _FakeTarget:
def __init__(self):
self.sendport_args = None
def voidcmd(self, cmd):
pass
def sendport(self, host, port):
self.sendport_args = (host, port)
def sendcmd(self, cmd):
return '150 ok'
def voidresp(self):
pass

def test_ftpcp_ignores_untrusted_pasv_host(self):
source = self._FakeSource('10.0.0.5', '198.51.100.7')
target = self._FakeTarget()
ftplib.ftpcp(source, 'a', target, 'b')
self.assertEqual(target.sendport_args, ('198.51.100.7', 258))

def test_ftpcp_trust_server_pasv_ipv4_address(self):
source = self._FakeSource('10.0.0.5', '198.51.100.7')
source.trust_server_pasv_ipv4_address = True
target = self._FakeTarget()
ftplib.ftpcp(source, 'a', target, 'b')
self.assertEqual(target.sendport_args, ('10.0.0.5', 258))


class MiscTestCase(TestCase):
def test__all__(self):
not_exported = {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
The :mod:`ftplib` module's undocumented ``ftpcp`` function no longer trusts
the IPv4 address value returned from the source server in response to the
``PASV`` command by default, completing the fix for CVE-2021-4189. As with
:class:`ftplib.FTP`, the former behavior can be re-enabled by setting the
``trust_server_pasv_ipv4_address`` attribute on the source :class:`ftplib.FTP`
instance to ``True``. Thanks to Qi Ding (AKA ikow) for the report.
Loading