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

netrc: free temporary strings if memory allocation fails #3122

Closed
wants to merge 1 commit into from
Closed

netrc: free temporary strings if memory allocation fails #3122

wants to merge 1 commit into from

Conversation

mkauf
Copy link
Contributor

@mkauf mkauf commented Oct 10, 2018

  • Change the inout parameters after all needed memory has been
    allocated. Do not change them if something goes wrong.
  • Free the allocated temporary strings if strdup() fails

@mkauf
netrc: free temporary strings if memory allocation fails
0c43259 
- Change the inout parameters after all needed memory has been
  allocated. Do not change them if something goes wrong.
- Free the allocated temporary strings if strdup() fails
@mkauf
Copy link
Contributor Author

mkauf commented Oct 13, 2018

This fixes a bug found by the torture tests: https://curl.haxx.se/dev/log.cgi?id=20181007082759-19055#prob1

** MEMORY FAILURE
Leak detected: memory still allocated: 6 bytes
At 991c594, there's 6 bytes.
allocated by ../../../../home/autobuild/curl/curl.git/lib/netrc.c:165
LIMIT ../../../../home/autobuild/curl/curl.git/lib/netrc.c:176 strdup reached memlimit

@mkauf mkauf closed this in d48e6b7 Oct 25, 2018
@mkauf mkauf deleted the netrc_mem_fix branch October 25, 2018 10:56
@alexcumbers
Copy link

There appears to be a bug with this change where netrc lookup now fails to select the correct entry, instead defaulting to the first user entry every time. This was working in v7.61.1 and earlier.

Example netrc file:
machine ftp.gam.com login inet_web password INCORRECT
machine ftp.gam.com login fdxfer password ----
machine ftp.gam.com login impldn password -----
machine ftp.gam.com login pharo password ----

Version:
curl 7.62.0 (sparc-sun-solaris2.10) libcurl/7.62.0 OpenSSL/1.0.2p zlib/1.2.11 libssh2/1.8.0
Release-Date: 2018-10-31
Protocols: dict file ftp ftps gopher http https imap imaps pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp
Features: AsynchDNS IPv6 Largefile NTLM NTLM_WB SSL libz TLS-SRP UnixSockets HTTPS-proxy

The following examples running under Solaris 11 pick up the wrong netrc user entry:

curl --verbose --netrc --user fdxfer: ftp://ftp.gam.com

  • Trying 193.202.226.107...
  • TCP_NODELAY set
  • Failed to set TCP_KEEPALIVE on fd 4
  • Connected to ftp.gam.com (193.202.226.107) port 21 (#0)
    < 220 ftp.gam.com FTP server ready.
    > USER inet_web
    < 331 Password required for inet_web.

PASS INCORRECT
< 530 Login incorrect.

curl --verbose --netrc --user pilling: ftp://ftp.gam.com

  • Trying 193.202.226.107...
  • TCP_NODELAY set
  • Failed to set TCP_KEEPALIVE on fd 4
  • Connected to ftp.gam.com (193.202.226.107) port 21 (#0)
    < 220 ftp.gam.com FTP server ready.
    > USER inet_web
    < 331 Password required for inet_web.

PASS INCORRECT
< 530 Login incorrect.

@alexcumbers alexcumbers mentioned this pull request Nov 1, 2018
Closed
@lock lock bot locked as resolved and limited conversation to collaborators Jan 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bagder bagder bagder approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mkauf @alexcumbers @bagder