Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

(ftplib) A remote attacker could possibly attack by containing the newline characters #74305

Closed
corona10 opened this issue Apr 20, 2017 · 20 comments
Closed

(ftplib) A remote attacker could possibly attack by containing the newline characters #74305

corona10 opened this issue Apr 20, 2017 · 20 comments
Labels
3.7 (EOL) end of life type-security A security issue

Comments

@corona10
Copy link
Member

BPO 30119
Nosy @birkenfeld, @vstinner, @larryhastings, @giampaolo, @benjaminp, @ned-deily, @vadmium, @corona10
PRs
  • bpo-30119: Fix ftplib to handle the ftp user's commands. #1214
  • [3.3] bpo-30119: fix ftplib.FTP.putline() to throw an error for a illegal command (#1214) #2885
  • [3.6] bpo-30119: fix ftplib.FTP.putline() to throw an error for a illegal command (#1214) #2886
  • [3.5] bpo-30119: fix ftplib.FTP.putline() to throw an error for a illegal command (#1214) #2887
  • [3.4] bpo-30119: fix ftplib.FTP.putline() to throw an error for a illegal command (#1214) #2893
  • [2.7] bpo-30119: fix ftplib.FTP.putline() to throw an error for a illegal command (#1214) #2894
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2017-07-28.04:07:50.206>
created_at = <Date 2017-04-20.17:57:20.239>
labels = ['type-security', '3.7']
title = '(ftplib) A remote attacker could possibly attack by containing the newline characters'
updated_at = <Date 2019-05-10.18:12:00.146>
user = 'https://github.com/corona10'

    bugs.python.org fields:

    activity = <Date 2019-05-10.18:12:00.146>
actor = 'ned.deily'
assignee = 'none'
closed = True
closed_date = <Date 2017-07-28.04:07:50.206>
closer = 'ned.deily'
components = []
creation = <Date 2017-04-20.17:57:20.239>
creator = 'corona10'
dependencies = []
files = []
hgrepos = []
issue_num = 30119
keywords = ['security_issue']
message_count = 20.0
messages = ['291988', '292556', '292557', '292591', '292693', '298860', '299141', '299182', '299199', '299204', '299205', '299209', '299212', '299225', '299226', '299228', '299229', '299230', '299242', '299341']
nosy_count = 8.0
nosy_names = ['georg.brandl', 'vstinner', 'larry', 'giampaolo.rodola', 'benjamin.peterson', 'ned.deily', 'martin.panter', 'corona10']
pr_nums = ['1214', '2885', '2886', '2887', '2893', '2894']
priority = None
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue30119'
versions = ['Python 2.7', 'Python 3.3', 'Python 3.4', 'Python 3.5', 'Python 3.6', 'Python 3.7']
    @corona10
    Copy link
    Member Author

    It was discovered that the FTP client implementation in the Networking component of Python failed to correctly handle user inputs.
    A remote attacker could possibly use this flaw to manipulate an FTP connection opened by a Python application if it could make it access a specially crafted FTP URL.

    See
    http://blog.blindspotsecurity.com/2017/02/advisory-javapython-ftp-injections.html

    and https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2017-3533

    I upload the patch for this issue.

    @corona10 corona10 added type-security A security issue 3.7 (EOL) end of life labels Apr 20, 2017
    @corona10 corona10 changed the title A remote attacker could possibly use this flaw to manipulate an FTP connection opened by a Python application (ftplib) A remote attacker could possibly attack by containing the newline characters Apr 20, 2017
    @corona10
    Copy link
    Member Author

    One of the purposes of the JDK patch is to prevent '\ r' and '\ n' from being inserted into the ftp command. In particular, it seems to assume that if another malice command is inserted after '\ n', the possibility of such an attack will be opened at a later time.
    IMO, I think that we can block '\ r \ n' and '\ n' at the same time by blocking only '\ n'. Although '\ r' allows

    @corona10
    Copy link
    Member Author

    '\ r' -> '\r'
    '\ n' -> '\n'

    @vadmium
    Copy link
    Member

    vadmium commented Apr 29, 2017

    I suggest to close this as a duplicate. The pull request itself looks like the right direction to me, but let’s not split the discussion up more than necessary.

    @giampaolo
    Copy link
    Contributor

    The relevant discussion of this bug is happening in #1214.

    @giampaolo
    Copy link
    Contributor

    New changeset 2b1e6e9 by Giampaolo Rodola (Dong-hee Na) in branch 'master':
    bpo-30119: fix ftplib.FTP.putline() to throw an error for a illegal command (bpo-1214)
    2b1e6e9

    @giampaolo
    Copy link
    Contributor

    Reopening as it needs backports for 2.7, 3.3, 3.4, 3.5 and 3.6.

    @giampaolo giampaolo reopened this Jul 25, 2017
    @vstinner
    Copy link
    Member

    What about rejecting also NUL byte?

    @vstinner
    Copy link
    Member

    I closed bpo-29606 as a duplicate of this bug.

    @ned-deily
    Copy link
    Member

    Just FYI, if the backports to 3.5, 3.4, and 3.3 happen *really* fast, we *might* be able to get them into the current round of releases, if Larry approves for 3.5.4 final and 3.4.7 final. If the 3.3 backport doesn't happen soon, 3.3 will reach end of life without it.

    @corona10
    Copy link
    Member Author

    Okay, I will send backport today.

    @ned-deily
    Copy link
    Member

    New changeset a4e774f by Ned Deily (Dong-hee Na) in branch '3.3':
    [3.3] bpo-30119: fix ftplib.FTP.putline() to throw an error for a illegal command (bpo-1214) (bpo-2885)
    a4e774f

    @ned-deily
    Copy link
    Member

    New changeset 19b2890 by Ned Deily (Dong-hee Na) in branch '3.5':
    [3.5] [security] bpo-30119: fix ftplib.FTP.putline() to throw an error for a illegal command (bpo-1214) (bpo-2887)
    19b2890

    @vstinner
    Copy link
    Member

    New changeset 8c2d4cf by Victor Stinner (Dong-hee Na) in branch '3.6':
    [3.6] bpo-30119: fix ftplib.FTP.putline() to throw an error for a illegal command (bpo-1214) (bpo-2886)
    8c2d4cf

    @vstinner
    Copy link
    Member

    @corona10: Cool, 3.3, 3.5, 3.6 and master are fixed. Would you mind to create also backports for 2.7 and 3.4, please?

    @giampaolo
    Copy link
    Contributor

    What about rejecting also NUL byte?

    I don't it would make any difference at this point.

    @vstinner
    Copy link
    Member

    Victor> What about rejecting also NUL byte?
    Giampaolo Rodola'> I don't it would make any difference at this point.

    I asked because I read that filenames containing newlines can be escaped using \n\0. So it seems like "embedded" NUL bytes have a special semantic in FTP.
    http://bugs.python.org/issue29606#msg292677

    I have no opinion on NUL bytes. It's just that I saw them mentionned somewhere in the discussion, but I failed to see a rationale to accept or reject them.

    @giampaolo
    Copy link
    Contributor

    AFAIK its only use case is to escape \r and \n.

    @vstinner
    Copy link
    Member

    New changeset e5eae47 by Victor Stinner (Dong-hee Na) in branch '2.7':
    [2.7] bpo-30119: fix ftplib.FTP.putline() to throw an error for a illegal command (bpo-1214) (bpo-2894)
    e5eae47

    @larryhastings
    Copy link
    Contributor

    New changeset 2a5a26c by larryhastings (Dong-hee Na) in branch '3.4':
    [3.4] bpo-30119: fix ftplib.FTP.putline() to throw an error for a illegal command (bpo-1214) (bpo-2893)
    2a5a26c

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.7 (EOL) end of life type-security A security issue
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    7 participants
    @berkerpeksag @vstinner @giampaolo @vadmium @corona10 @ned-deily @larryhastings