Skip to content

Commit bb3446d

Browse files
[3.13] gh-87451: Apply CVE-2021-4189 PASV fix to ftplib.ftpcp() (GH-149648) (#149794)
gh-87451: Apply CVE-2021-4189 PASV fix to ftplib.ftpcp() (GH-149648) 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 at Aurascape AI for the report. (GHSA-w8c5-q2xf-gf7c) (cherry picked from commit eac4fe3) Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com>
1 parent 49d0867 commit bb3446d

3 files changed

Lines changed: 51 additions & 2 deletions

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: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
except ImportError:
1717
ssl = None
1818

19-
from unittest import TestCase, skipUnless
19+
from unittest import mock, TestCase, skipUnless
2020
from test import support
2121
from test.support import requires_subprocess
2222
from test.support import threading_helper
@@ -1145,6 +1145,40 @@ 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+
def _make_pair(self, *, advertised_host, real_host, trust=False):
1157+
source = mock.Mock(spec=ftplib.FTP)
1158+
source.trust_server_pasv_ipv4_address = trust
1159+
source.sock.getpeername.return_value = (real_host, 21)
1160+
# PASV replies give the host as comma-separated octets, not dotted.
1161+
advertised = advertised_host.replace('.', ',')
1162+
source.sendcmd.side_effect = lambda cmd: (
1163+
f'227 Entering Passive Mode ({advertised},1,2).'
1164+
if cmd == 'PASV' else '150 ok')
1165+
target = mock.Mock(spec=ftplib.FTP)
1166+
target.sendcmd.return_value = '150 ok'
1167+
return source, target
1168+
1169+
def test_ftpcp_ignores_untrusted_pasv_host(self):
1170+
source, target = self._make_pair(advertised_host='10.0.0.5',
1171+
real_host='198.51.100.7')
1172+
ftplib.ftpcp(source, 'a', target, 'b')
1173+
target.sendport.assert_called_once_with('198.51.100.7', 258)
1174+
1175+
def test_ftpcp_trust_server_pasv_ipv4_address(self):
1176+
source, target = self._make_pair(advertised_host='10.0.0.5',
1177+
real_host='198.51.100.7', trust=True)
1178+
ftplib.ftpcp(source, 'a', target, 'b')
1179+
target.sendport.assert_called_once_with('10.0.0.5', 258)
1180+
1181+
11481182
class MiscTestCase(TestCase):
11491183
def test__all__(self):
11501184
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 Deng at Aurascape AI for the report.

0 commit comments

Comments
 (0)