Re: PATCH: Curl Sanity patch for spnego authentication

[Daniel Stenberg]

On Mon, 16 Sep 2013, Arunav Sanyal wrote:

Ok daniel here is the change:-

Thanks. I've posted my updated version of your fix that now follows our coding 
style.

My only concern with the added ifdefs for the memory handling is that the 
source code seems to imply that it can work with both gssapi and spnego 
present at the same time and then it isn't correct to select the method at 
compile-time but it should instead be decided in run-time.

Do you agree?

--

 / daniel.haxx.seFrom 7b3ab4e88492057d5423df4a70a2d1f92b7cb3eb Mon Sep 17 00:00:00 2001
From: Arunav Sanyal <arunav.sanya...@gmail.com>
Date: Wed, 18 Sep 2013 00:26:18 +0200
Subject: [PATCH] spnego: fix crash due to incorrect free

Using curl with fbopenssl crashed the program due to incorrect memory
free operations.

Explanation - fbopenssl internally did malloc and the free operation
attempted gss_release_buffer(). Fbopenssl internal documentation expects
free operation of type free()
---
 lib/http_negotiate.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/lib/http_negotiate.c b/lib/http_negotiate.c
index 9b981b3..4064797 100644
--- a/lib/http_negotiate.c
+++ b/lib/http_negotiate.c
@@ -314,7 +314,7 @@ CURLcode Curl_output_negotiate(struct connectdata *conn, bool proxy)
       Curl_safefree(responseToken);
       ASN1_OBJECT_free(object);
       if(spnegoToken.value)
-        gss_release_buffer(&discard_st, &spnegoToken);
+        Curl_safefree(spnegoToken.value);
       infof(conn->data, "Make SPNEGO Initial Token succeeded (NULL token)\n");
     }
     else {
@@ -332,14 +332,22 @@ CURLcode Curl_output_negotiate(struct connectdata *conn, bool proxy)
                              neg_ctx->output_token.length,
                              &encoded, &len);
   if(error) {
+#ifdef HAVE_SPNEGO
+    Curl_safefree(neg_ctx->output_token.value);
+#else
     gss_release_buffer(&discard_st, &neg_ctx->output_token);
+#endif
     neg_ctx->output_token.value = NULL;
     neg_ctx->output_token.length = 0;
     return error;
   }
 
   if(!encoded || !len) {
+#ifdef HAVE_SPNEGO
+    Curl_safefree(neg_ctx->output_token.value);
+#else
     gss_release_buffer(&discard_st, &neg_ctx->output_token);
+#endif
     neg_ctx->output_token.value = NULL;
     neg_ctx->output_token.length = 0;
     return CURLE_REMOTE_ACCESS_DENIED;
@@ -368,8 +376,14 @@ static void cleanup(struct negotiatedata *neg_ctx)
   if(neg_ctx->context != GSS_C_NO_CONTEXT)
     gss_delete_sec_context(&minor_status, &neg_ctx->context, GSS_C_NO_BUFFER);
 
-  if(neg_ctx->output_token.value)
-    gss_release_buffer(&minor_status, &neg_ctx->output_token);
+  if(neg_ctx->output_token.value) {
+#ifdef HAVE_SPNEGO
+    Curl_safefree(neg_ctx->output_token.value);
+#else
+    if(neg_ctx->output_token.value)
+      gss_release_buffer(&minor_status, &neg_ctx->output_token);
+#endif
+  }
 
   if(neg_ctx->server_name != GSS_C_NO_NAME)
     gss_release_name(&minor_status, &neg_ctx->server_name);
-- 
1.8.4.rc3

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html