Issue 43285: ftplib should not use the host from the PASV response

Last year, curl had a security update for CVE-2020-8284. more info, see https://hackerone.com/reports/1040166

The problem is ftp client trust the host from PASV response by default, A malicious server can trick ftp client into connecting
back to a given IP address and port. This may make ftp client scan ports and extract service banner from private newwork.

After test and read ftplib module(https://github.com/python/cpython/blob/63298930fb531ba2bb4f23bc3b915dbf1e17e9e1/Lib/ftplib.py#L346), I found ftplib has the same problem.

Any response here? If you need more information let me know.

Indeed, the `host` on that line there should just be ignored with the IP address of the original data connection used in its place.

Your https://hackerone.com/reports/1040166 link provides plenty of information and likes to prior art mitigations other ftp clients including Firefox and Chrome well over a decade ago.

I'm not interested in chasing down a CVE for this myself.  If anyone wants to jump through the hoops to obtain one, the text used for curl in the hackerone link is likely a good guide.

My PR includes a way for people to opt-out of the secure behavior (why would anyone ever want that?) by setting the use_untrusted_server_pasv_ipv4_addr attribute to True on their ftplib.FTP instance.  Setting that attribute on a server lacking this fix is a no-op, making it safe to add to code running on any version.

This is an embarrassingly old widespread common issue in a large number of ftp clients.  Even the 1998 IPv6 RFC https://tools.ietf.org/html/rfc2428 indirectly acknowledges its existence by disallowing the new EPSV command that replaces PASV from returning anything other than the port number while leaving fields for the other values present but empty...

New changeset 0ab152c6b5d95caa2dc1a30fa96e10258b5f188e by Gregory P. Smith in branch 'master':
bpo-43285 Make ftplib not trust the PASV response. (GH-24838)
https://github.com/python/cpython/commit/0ab152c6b5d95caa2dc1a30fa96e10258b5f188e

New changeset 7dcb4baa4f0fde3aef5122a8e9f6a41853ec9335 by Miss Islington (bot) in branch '3.9':
bpo-43285 Make ftplib not trust the PASV response. (GH-24838)
https://github.com/python/cpython/commit/7dcb4baa4f0fde3aef5122a8e9f6a41853ec9335

New changeset 664d1d16274b47eea6ec92572e1ebf3939a6fa0c by Gregory P. Smith in branch '3.8':
[3.8] bpo-43285 Make ftplib not trust the PASV response. (GH-24838) (GH-24881)
https://github.com/python/cpython/commit/664d1d16274b47eea6ec92572e1ebf3939a6fa0c

3.7 and 3.6 backport PRs created and assigned to release manager Ned for merging.

@gps, What about ftplib doc changes and What's new entries for this change in behavior?

A What's New entry is a good idea.  I'll make one and add it to those backport PRs.  (reopened to remind me of that)

ftplib docs... I don't actually want to document the attribute that people can set for the old behavior beyond the notes in NEWS or What's New.  It is something I anticipate nobody in the world ever actually setting so I'd rather not imply that anyone even should by giving it more prominent doc space.

Other things that have fixed this repeated bug in their program that supports ftp over the years have not added an opt-out as far as I could tell in my quick searching.

New changeset d0312cece9ce89d783687ff6dddaae6495e19fcf by Gregory P. Smith in branch '3.9':
[3.9] bpo-43285: Add a What's New entry for 3.9.3. (GH-24888)
https://github.com/python/cpython/commit/d0312cece9ce89d783687ff6dddaae6495e19fcf

New changeset 9eda0dfff2884bf9272f37d4151ef2335f55066f by Gregory P. Smith in branch '3.8':
[3.8] bpo-43285: Whats New entry for 3.8.9. (GH-24889)
https://github.com/python/cpython/commit/9eda0dfff2884bf9272f37d4151ef2335f55066f

3.7 and 3.6 PRs updated to include a What's New entry.

New changeset 4134f154ae2f621f25c5d698cc0f1748035a1b88 by Miss Islington (bot) in branch '3.6':
[3.6] bpo-43285 Make ftplib not trust the PASV response. (GH-24838) (GH-24881) (GH-24882)
https://github.com/python/cpython/commit/4134f154ae2f621f25c5d698cc0f1748035a1b88

New changeset 79373951b3eab585d42e0f0ab83718cbe1d0ee33 by Miss Islington (bot) in branch '3.7':
[3.7] bpo-43285 Make ftplib not trust the PASV response. (GH-24838) (GH-24881) (GH-24883)
https://github.com/python/cpython/commit/79373951b3eab585d42e0f0ab83718cbe1d0ee33

Thanks for the PRs and the What's New entries.

stage: patch review -> resolved

versions: - Python 3.8, Python 3.9, Python 3.10

messages: + msg388777

nosy: + ned.deily
messages: + msg388763

resolution: fixed
stage: patch review -> commit review

messages: + msg388602

assignee: gregory.p.smith
stage: needs patch