From 3be7f4059e0e13940a78d8507e6cedb7613386e4 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sun, 10 May 2026 18:19:12 +0000 Subject: [PATCH 1/3] 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. --- Lib/ftplib.py | 11 +++- Lib/test/test_ftplib.py | 54 +++++++++++++++++++ ...6-05-10-18-05-32.gh-issue-87451.XkKB6M.rst | 6 +++ 3 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Security/2026-05-10-18-05-32.gh-issue-87451.XkKB6M.rst diff --git a/Lib/ftplib.py b/Lib/ftplib.py index 640acc64f620cc..2f092d50f31782 100644 --- a/Lib/ftplib.py +++ b/Lib/ftplib.py @@ -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. diff --git a/Lib/test/test_ftplib.py b/Lib/test/test_ftplib.py index c864d401f9ed67..83a7bfd390f5d7 100644 --- a/Lib/test/test_ftplib.py +++ b/Lib/test/test_ftplib.py @@ -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: + 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 = { diff --git a/Misc/NEWS.d/next/Security/2026-05-10-18-05-32.gh-issue-87451.XkKB6M.rst b/Misc/NEWS.d/next/Security/2026-05-10-18-05-32.gh-issue-87451.XkKB6M.rst new file mode 100644 index 00000000000000..034b877c923579 --- /dev/null +++ b/Misc/NEWS.d/next/Security/2026-05-10-18-05-32.gh-issue-87451.XkKB6M.rst @@ -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. From 19999b85f76bad5861e409076abb7f458d26e47d Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Wed, 13 May 2026 09:47:01 -0700 Subject: [PATCH 2/3] Update the thank you attribution. --- .../next/Security/2026-05-10-18-05-32.gh-issue-87451.XkKB6M.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Security/2026-05-10-18-05-32.gh-issue-87451.XkKB6M.rst b/Misc/NEWS.d/next/Security/2026-05-10-18-05-32.gh-issue-87451.XkKB6M.rst index 034b877c923579..21a79c3e0e7db7 100644 --- a/Misc/NEWS.d/next/Security/2026-05-10-18-05-32.gh-issue-87451.XkKB6M.rst +++ b/Misc/NEWS.d/next/Security/2026-05-10-18-05-32.gh-issue-87451.XkKB6M.rst @@ -3,4 +3,4 @@ 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. +instance to ``True``. Thanks to Qi Deng at Aurascape AI for the report. From 917aad965803d44f001fe0e8aa48637268688f6d Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Wed, 13 May 2026 09:53:32 -0700 Subject: [PATCH 3/3] gh-87451: Use unittest.mock instead of hand-written fakes in ftpcp test Replace the _Fake* helper classes in TestFtpcpSecurity with mock.Mock objects spec'd against ftplib.FTP. The spec gives a real-API check that ftpcp() only touches attributes that exist on FTP, and a keyword-only _make_pair() helper keeps the call sites self-documenting. --- Lib/test/test_ftplib.py | 58 ++++++++++++++--------------------------- 1 file changed, 19 insertions(+), 39 deletions(-) diff --git a/Lib/test/test_ftplib.py b/Lib/test/test_ftplib.py index 83a7bfd390f5d7..f1eff9430f7351 100644 --- a/Lib/test/test_ftplib.py +++ b/Lib/test/test_ftplib.py @@ -16,7 +16,7 @@ except ImportError: ssl = None -from unittest import TestCase, skipUnless +from unittest import mock, TestCase, skipUnless from test import support from test.support import requires_subprocess from test.support import threading_helper @@ -1153,50 +1153,30 @@ class TestFtpcpSecurity(TestCase): source server's actual peer address instead, the same as FTP.makepasv(). """ - class _FakeSock: - 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 _make_pair(self, *, advertised_host, real_host, trust=False): + source = mock.Mock(spec=ftplib.FTP) + source.trust_server_pasv_ipv4_address = trust + source.sock.getpeername.return_value = (real_host, 21) + # PASV replies give the host as comma-separated octets, not dotted. + advertised = advertised_host.replace('.', ',') + source.sendcmd.side_effect = lambda cmd: ( + f'227 Entering Passive Mode ({advertised},1,2).' + if cmd == 'PASV' else '150 ok') + target = mock.Mock(spec=ftplib.FTP) + target.sendcmd.return_value = '150 ok' + return source, target def test_ftpcp_ignores_untrusted_pasv_host(self): - source = self._FakeSource('10.0.0.5', '198.51.100.7') - target = self._FakeTarget() + source, target = self._make_pair(advertised_host='10.0.0.5', + real_host='198.51.100.7') ftplib.ftpcp(source, 'a', target, 'b') - self.assertEqual(target.sendport_args, ('198.51.100.7', 258)) + target.sendport.assert_called_once_with('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() + source, target = self._make_pair(advertised_host='10.0.0.5', + real_host='198.51.100.7', trust=True) ftplib.ftpcp(source, 'a', target, 'b') - self.assertEqual(target.sendport_args, ('10.0.0.5', 258)) + target.sendport.assert_called_once_with('10.0.0.5', 258) class MiscTestCase(TestCase):