Skip to content

netrc: free temporary strings if memory allocation fails#3122

Closed
mkauf wants to merge 1 commit intocurl:masterfrom
mkauf:netrc_mem_fix
Closed

netrc: free temporary strings if memory allocation fails#3122
mkauf wants to merge 1 commit intocurl:masterfrom
mkauf:netrc_mem_fix

Conversation

@mkauf
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

alexcumbers commented Nov 1, 2018

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.

3 participants

@mkauf @alexcumbers @bagder