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
Memory Leak Observed in libcurl With Negotiate (Kerberos) Failure case #1115
Comments
Hi @ashman-p, I could reproduce the leak with the below curl command:
I can confirm your fix does solve the leak problem, but perhaps we could instead simply cleanup the negotiate data immediately right when it fails, something like: $ git diff
diff --git a/lib/vauth/spnego_gssapi.c b/lib/vauth/spnego_gssapi.c
index 8840db8..213f4eb 100644
--- a/lib/vauth/spnego_gssapi.c
+++ b/lib/vauth/spnego_gssapi.c
@@ -169,6 +169,7 @@ CURLcode Curl_auth_decode_spnego_message(struct Curl_easy *data,
Curl_gss_log_error(data, "gss_init_sec_context() failed: ",
major_status, minor_status);
+ Curl_auth_spnego_cleanup(nego);
return CURLE_OUT_OF_MEMORY;
} What do you think? |
Thanks very much. I like your solution much better. Very nice. On Nov 8, 2016, at 3:19 PM, Isaac Boukris <notifications@github.commailto:notifications@github.com> wrote: Hi @ashman-phttps://github.com/ashman-p, I could reproduce the leak with the below curl command: $ valgrind --leak-check=full curl -v -u: --negotiate http://ms.frenche.cp/mika/
I can confirm your fix does solve the leak problem, but perhaps we could simply cleanup the negotiate data immediately right when it fails, something like: $ git diff
return CURLE_OUT_OF_MEMORY; What do you think? — |
Oh, I think this might be needed for windows SSPI as well, so maybe we should move the cleanup logic to the caller, see below: diff --git a/lib/http_negotiate.c b/lib/http_negotiate.c
index eb17ed4..5ea9f34 100644
--- a/lib/http_negotiate.c
+++ b/lib/http_negotiate.c
@@ -37,6 +37,7 @@
CURLcode Curl_input_negotiate(struct connectdata *conn, bool proxy,
const char *header)
{
+ CURLcode result;
struct Curl_easy *data = conn->data;
size_t len;
@@ -89,8 +90,13 @@ CURLcode Curl_input_negotiate(struct connectdata *conn, bool proxy,
}
/* Initilise the security context and decode our challenge */
- return Curl_auth_decode_spnego_message(data, userp, passwdp, service, host,
- header, neg_ctx);
+ result = Curl_auth_decode_spnego_message(data, userp, passwdp, service,
+ host, header, neg_ctx);
+
+ if(result)
+ Curl_auth_spnego_cleanup(neg_ctx);
+
+ return result;
}
CURLcode Curl_output_negotiate(struct connectdata *conn, bool proxy) Alternatively, we can simply do the same cleanup in spnego_sspi.c file as well. |
If SPNEGO fails, cleanup the negotiate handle right away. Fixes curl#1115 Signed-off-by: Isaac Boukris <iboukris@gmail.com> Reported-by: ashman-p
I did this
Access a web -site protected with Kerberos but, the client krb ticket is not available.
I expected the following
Access was rejected as expected but, valgrind reported a memory leak.
The leak seems to occur because Curl_http_done() is not called if the context init fails when there is no ticket (or it has expired???).
curl/libcurl version
curl-7.50.3
(and earlier versions as well)
[curl -V output perhaps?]
curl 7.50.3 (x86_64-pc-linux-gnu) libcurl/7.50.3 OpenSSL/1.0.2h zlib/1.2.7
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: IPv6 Largefile GSS-API Kerberos SPNEGO NTLM NTLM_WB SSL libz TLS-SRP UnixSockets
operating system
Centos
Test App Src that can recreate the issue
int main(int argc, char* argv[])
{
CURLcode result;
int x;
char url[1024];
}
Patch that is a possible fix
Attached....
http.c.curl-7.50.patch.txt
The text was updated successfully, but these errors were encountered: