Skip to content

Commit 3be7f40

Browse files
committed
gh-87451: Apply CVE-2021-4189 PASV fix to ftplib.ftpcp()
ftpcp() called parse227() directly and passed the source server's self-reported PASV IPv4 address to the target server's PORT command, bypassing the CVE-2021-4189 fix that was applied only to FTP.makepasv(). A malicious source FTP server could use this to redirect the target server's data connection to an arbitrary host:port (SSRF). ftpcp() now uses the source server's actual peer address, honoring the existing trust_server_pasv_ipv4_address opt-out, the same as makepasv(). Thanks to Qi Ding (AKA ikow) for the report.
1 parent 85d3bcd commit 3be7f40

3 files changed

Lines changed: 70 additions & 1 deletion

File tree

Lib/ftplib.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -883,7 +883,16 @@ def ftpcp(source, sourcename, target, targetname = '', type = 'I'):
883883
type = 'TYPE ' + type
884884
source.voidcmd(type)
885885
target.voidcmd(type)
886-
sourcehost, sourceport = parse227(source.sendcmd('PASV'))
886+
# Don't trust the IPv4 address the source server advertises in its PASV
887+
# reply: a malicious source could otherwise point the target's data
888+
# connection at an arbitrary host (SSRF). A caller that needs the old
889+
# behavior can set trust_server_pasv_ipv4_address on the source FTP
890+
# object. See FTP.makepasv(), which applies the same rule.
891+
untrusted_host, sourceport = parse227(source.sendcmd('PASV'))
892+
if source.trust_server_pasv_ipv4_address:
893+
sourcehost = untrusted_host
894+
else:
895+
sourcehost = source.sock.getpeername()[0]
887896
target.sendport(sourcehost, sourceport)
888897
# RFC 959: the user must "listen" [...] BEFORE sending the
889898
# transfer request.

Lib/test/test_ftplib.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1145,6 +1145,60 @@ def testTimeoutDirectAccess(self):
11451145
ftp.close()
11461146

11471147

1148+
class TestFtpcpSecurity(TestCase):
1149+
"""ftpcp() must not trust the host a source server advertises in PASV.
1150+
1151+
A malicious source server can otherwise redirect the target server's
1152+
data connection to an arbitrary host:port (SSRF), so ftpcp() uses the
1153+
source server's actual peer address instead, the same as FTP.makepasv().
1154+
"""
1155+
1156+
class _FakeSock:
1157+
def __init__(self, peer_host):
1158+
self._peer = (peer_host, 21)
1159+
def getpeername(self):
1160+
return self._peer
1161+
1162+
class _FakeSource:
1163+
trust_server_pasv_ipv4_address = False
1164+
def __init__(self, advertised_host, real_host):
1165+
self.sock = TestFtpcpSecurity._FakeSock(real_host)
1166+
self._advertised = advertised_host.replace('.', ',')
1167+
def voidcmd(self, cmd):
1168+
pass
1169+
def sendcmd(self, cmd):
1170+
if cmd == 'PASV':
1171+
return '227 Entering Passive Mode (%s,1,2).' % self._advertised
1172+
return '150 ok'
1173+
def voidresp(self):
1174+
pass
1175+
1176+
class _FakeTarget:
1177+
def __init__(self):
1178+
self.sendport_args = None
1179+
def voidcmd(self, cmd):
1180+
pass
1181+
def sendport(self, host, port):
1182+
self.sendport_args = (host, port)
1183+
def sendcmd(self, cmd):
1184+
return '150 ok'
1185+
def voidresp(self):
1186+
pass
1187+
1188+
def test_ftpcp_ignores_untrusted_pasv_host(self):
1189+
source = self._FakeSource('10.0.0.5', '198.51.100.7')
1190+
target = self._FakeTarget()
1191+
ftplib.ftpcp(source, 'a', target, 'b')
1192+
self.assertEqual(target.sendport_args, ('198.51.100.7', 258))
1193+
1194+
def test_ftpcp_trust_server_pasv_ipv4_address(self):
1195+
source = self._FakeSource('10.0.0.5', '198.51.100.7')
1196+
source.trust_server_pasv_ipv4_address = True
1197+
target = self._FakeTarget()
1198+
ftplib.ftpcp(source, 'a', target, 'b')
1199+
self.assertEqual(target.sendport_args, ('10.0.0.5', 258))
1200+
1201+
11481202
class MiscTestCase(TestCase):
11491203
def test__all__(self):
11501204
not_exported = {
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
The :mod:`ftplib` module's undocumented ``ftpcp`` function no longer trusts
2+
the IPv4 address value returned from the source server in response to the
3+
``PASV`` command by default, completing the fix for CVE-2021-4189. As with
4+
:class:`ftplib.FTP`, the former behavior can be re-enabled by setting the
5+
``trust_server_pasv_ipv4_address`` attribute on the source :class:`ftplib.FTP`
6+
instance to ``True``. Thanks to Qi Ding (AKA ikow) for the report.

0 commit comments

Comments
 (0)