Package: libcurl3
Version: 7.31.0-2
Tags: upstream patch
X-Debbugs-Cc: Colby Ranger <cran...@google.com>

libcurl truncates passwords to 255 characters when sending them with
basic authentication.

Test case:

  # Prepare a long (300-character) password.
  s=0123456789
  s=$s$s$s$s$s$s$s$s$s$s
  s=$s$s$s

  # Start a server.
  nc -l -p 8888 | tee out &
  pid=$!

  # Ask curl to pass a long password to that server.
  curl --user me:$s http://localhost:8888 &
  sleep 1
  kill $pid

  # Extract the password.
  userpass=$(
        awk '/Authorization: Basic/ {print $3}' <out |
        tr -d '\r' |
        base64 -d
  )
  password=${userpass#me:}
  echo ${#password}

Expected result: 300
Actual result: 255

Noticed by using git with a long password.  Git sets the password with

        curl_easy_setopt(result, CURLOPT_PASSWORD, http_auth.password);

The value is copied into a 256-byte buffer allocated on the stack in
lib/url.c::create_conn():

  if(data->set.str[STRING_PASSWORD]) {
    strncpy(passwd, data->set.str[STRING_PASSWORD], MAX_CURL_PASSWORD_LENGTH);
    passwd[MAX_CURL_PASSWORD_LENGTH - 1] = '\0'; /* To be on safe side */
  }

This doesn't affect the return value from curl_easy_setopt.  From the
caller's point of view, there is no sign anything strange has
happened, except that authentication fails.

Thanks to Colby Ranger for the analysis.  How about something like
this patch?
---
 lib/url.c | 108 +++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 58 insertions(+), 50 deletions(-)

diff --git a/lib/url.c b/lib/url.c
index 7cec5bcc..ac9bf162 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -144,7 +144,7 @@ static void signalPipeClose(struct curl_llist *pipeline, 
bool pipe_broke);
 static CURLcode do_init(struct connectdata *conn);
 static CURLcode parse_url_login(struct SessionHandle *data,
                                 struct connectdata *conn,
-                                char *user, char *passwd, char *options);
+                                char **user, char **passwd, char **options);
 static CURLcode parse_login_details(const char *login, const size_t len,
                                     char **userptr, char **passwdptr,
                                     char **optionsptr);
@@ -3687,7 +3687,8 @@ static CURLcode findprotocol(struct SessionHandle *data,
 static CURLcode parseurlandfillconn(struct SessionHandle *data,
                                     struct connectdata *conn,
                                     bool *prot_missing,
-                                    char *user, char *passwd, char *options)
+                                    char **userp, char **passwdp,
+                                    char **optionsp)
 {
   char *at;
   char *fragment;
@@ -3931,7 +3932,7 @@ static CURLcode parseurlandfillconn(struct SessionHandle 
*data,
    * Parse the login details from the URL and strip them out of
    * the host name
    */
-  result = parse_url_login(data, conn, user, passwd, options);
+  result = parse_url_login(data, conn, userp, passwdp, optionsp);
   if(result != CURLE_OK)
     return result;
 
@@ -4411,7 +4412,7 @@ static CURLcode parse_proxy_auth(struct SessionHandle 
*data,
  */
 static CURLcode parse_url_login(struct SessionHandle *data,
                                 struct connectdata *conn,
-                                char *user, char *passwd, char *options)
+                                char **user, char **passwd, char **options)
 {
   CURLcode result = CURLE_OK;
   char *userp = NULL;
@@ -4428,9 +4429,9 @@ static CURLcode parse_url_login(struct SessionHandle 
*data,
   char *ptr = strchr(conn->host.name, '@');
   char *login = conn->host.name;
 
-  user[0] = 0;   /* to make everything well-defined */
-  passwd[0] = 0;
-  options[0] = 0;
+  DEBUGASSERT(*user == NULL);
+  DEBUGASSERT(*passwd == NULL);
+  DEBUGASSERT(*options == NULL);
 
   /* We will now try to extract the
    * possible login information in a string like:
@@ -4467,10 +4468,7 @@ static CURLcode parse_url_login(struct SessionHandle 
*data,
             return CURLE_OUT_OF_MEMORY;
           }
 
-          if(strlen(newname) < MAX_CURL_USER_LENGTH)
-            strcpy(user, newname);
-
-          free(newname);
+          *user = newname;
         }
 
         if(passwdp) {
@@ -4483,10 +4481,7 @@ static CURLcode parse_url_login(struct SessionHandle 
*data,
             return CURLE_OUT_OF_MEMORY;
           }
 
-          if(strlen(newpasswd) < MAX_CURL_PASSWORD_LENGTH)
-            strcpy(passwd, newpasswd);
-
-          free(newpasswd);
+          *passwd = newpasswd;
         }
 
         if(optionsp) {
@@ -4499,10 +4494,7 @@ static CURLcode parse_url_login(struct SessionHandle 
*data,
             return CURLE_OUT_OF_MEMORY;
           }
 
-          if(strlen(newoptions) < MAX_CURL_OPTIONS_LENGTH)
-            strcpy(options, newoptions);
-
-          free(newoptions);
+          *options = newoptions;
         }
       }
 
@@ -5018,9 +5010,9 @@ static CURLcode create_conn(struct SessionHandle *data,
   struct connectdata *conn;
   struct connectdata *conn_temp = NULL;
   size_t urllen;
-  char user[MAX_CURL_USER_LENGTH];
-  char passwd[MAX_CURL_PASSWORD_LENGTH];
-  char options[MAX_CURL_OPTIONS_LENGTH];
+  char *user = NULL;
+  char *passwd = NULL;
+  char *options = NULL;
   bool reuse;
   char *proxy = NULL;
   bool prot_missing = FALSE;
@@ -5035,8 +5027,10 @@ static CURLcode create_conn(struct SessionHandle *data,
    * Check input data
    *************************************************************/
 
-  if(!data->change.url)
-    return CURLE_URL_MALFORMAT;
+  if(!data->change.url) {
+    result = CURLE_URL_MALFORMAT;
+    goto done;
+  }
 
   /* First, split up the current URL in parts so that we can use the
      parts for checking against the already present connections. In order
@@ -5044,8 +5038,10 @@ static CURLcode create_conn(struct SessionHandle *data,
      connection data struct and fill in for comparison purposes. */
   conn = allocate_conn(data);
 
-  if(!conn)
-    return CURLE_OUT_OF_MEMORY;
+  if(!conn) {
+    result = CURLE_OUT_OF_MEMORY;
+    goto done;
+  }
 
   /* We must set the return variable as soon as possible, so that our
      parent can cleanup any possible allocs we may have done before
@@ -5075,24 +5071,27 @@ static CURLcode create_conn(struct SessionHandle *data,
   data->state.path = NULL;
 
   data->state.pathbuffer = malloc(urllen+2);
-  if(NULL == data->state.pathbuffer)
-    return CURLE_OUT_OF_MEMORY; /* really bad error */
+  if(NULL == data->state.pathbuffer) {
+    result = CURLE_OUT_OF_MEMORY; /* really bad error */
+    goto done;
+  }
   data->state.path = data->state.pathbuffer;
 
   conn->host.rawalloc = malloc(urllen+2);
   if(NULL == conn->host.rawalloc) {
     Curl_safefree(data->state.pathbuffer);
     data->state.path = NULL;
-    return CURLE_OUT_OF_MEMORY;
+    result = CURLE_OUT_OF_MEMORY;
+    goto done;
   }
 
   conn->host.name = conn->host.rawalloc;
   conn->host.name[0] = 0;
 
-  result = parseurlandfillconn(data, conn, &prot_missing, user, passwd,
-                               options);
+  result = parseurlandfillconn(data, conn, &prot_missing, &user, &passwd,
+                               &options);
   if(result != CURLE_OK)
-    return result;
+    goto done;
 
   /*************************************************************
    * No protocol part in URL was used, add it!
@@ -5106,8 +5105,8 @@ static CURLcode create_conn(struct SessionHandle *data,
     reurl = aprintf("%s://%s", conn->handler->scheme, data->change.url);
 
     if(!reurl) {
-      Curl_safefree(proxy);
-      return CURLE_OUT_OF_MEMORY;
+      result = CURLE_OUT_OF_MEMORY;
+      goto done;
     }
 
     if(data->change.url_alloc) {
@@ -5144,7 +5143,7 @@ static CURLcode create_conn(struct SessionHandle *data,
   if(conn->bits.proxy_user_passwd) {
     result = parse_proxy_auth(data, conn);
     if(result != CURLE_OK)
-      return result;
+      goto done;
   }
 
   /*************************************************************
@@ -5155,7 +5154,8 @@ static CURLcode create_conn(struct SessionHandle *data,
     /* if global proxy is set, this is it */
     if(NULL == proxy) {
       failf(data, "memory shortage");
-      return CURLE_OUT_OF_MEMORY;
+      result = CURLE_OUT_OF_MEMORY;
+      goto done;
     }
   }
 
@@ -5186,13 +5186,14 @@ static CURLcode create_conn(struct SessionHandle *data,
     free(proxy); /* parse_proxy copies the proxy string */
 
     if(result)
-      return result;
+      goto done;
 
     if((conn->proxytype == CURLPROXY_HTTP) ||
        (conn->proxytype == CURLPROXY_HTTP_1_0)) {
 #ifdef CURL_DISABLE_HTTP
       /* asking for a HTTP proxy is a bit funny when HTTP is disabled... */
-      return CURLE_UNSUPPORTED_PROTOCOL;
+      result = CURLE_UNSUPPORTED_PROTOCOL;
+      goto done;
 #else
       /* force this connection's protocol to become HTTP if not already
          compatible - if it isn't tunneling through */
@@ -5222,10 +5223,8 @@ static CURLcode create_conn(struct SessionHandle *data,
    * we figured out what/if proxy to use.
    *************************************************************/
   result = setup_connection_internals(conn);
-  if(result != CURLE_OK) {
-    Curl_safefree(proxy);
-    return result;
-  }
+  if(result != CURLE_OK)
+    goto done;
 
   conn->recv[FIRSTSOCKET] = Curl_recv_plain;
   conn->send[FIRSTSOCKET] = Curl_send_plain;
@@ -5258,7 +5257,7 @@ static CURLcode create_conn(struct SessionHandle *data,
         DEBUGASSERT(conn->handler->done);
         /* we ignore the return code for the protocol-specific DONE */
         (void)conn->handler->done(conn, result, FALSE);
-        return result;
+        goto done;
       }
 
       Curl_setup_transfer(conn, -1, -1, FALSE, NULL, /* no download */
@@ -5268,7 +5267,7 @@ static CURLcode create_conn(struct SessionHandle *data,
     /* since we skip do_init() */
     Curl_speedinit(data);
 
-    return result;
+    goto done;
   }
 #endif
 
@@ -5284,13 +5283,13 @@ static CURLcode create_conn(struct SessionHandle *data,
    *************************************************************/
   result = parse_remote_port(data, conn);
   if(result != CURLE_OK)
-    return result;
+    goto done;
 
   /* Check for overridden login details and set them accordingly */
   override_login(data, conn, user, passwd, options);
   result = set_login(conn, user, passwd, options);
   if(result != CURLE_OK)
-    return result;
+    goto done;
 
   /* Get a cloned copy of the SSL config situation stored in the
      connection struct. But to get this going nicely, we must first make
@@ -5313,8 +5312,10 @@ static CURLcode create_conn(struct SessionHandle *data,
   data->set.ssl.password = data->set.str[STRING_TLSAUTH_PASSWORD];
 #endif
 
-  if(!Curl_clone_ssl_config(&data->set.ssl, &conn->ssl_config))
-    return CURLE_OUT_OF_MEMORY;
+  if(!Curl_clone_ssl_config(&data->set.ssl, &conn->ssl_config)) {
+    result = CURLE_OUT_OF_MEMORY;
+    goto done;
+  }
 
   /*************************************************************
    * Check the current list of connections to see if we can
@@ -5417,7 +5418,8 @@ static CURLcode create_conn(struct SessionHandle *data,
       conn_free(conn);
       *in_connect = NULL;
 
-      return CURLE_NO_CONNECTION_AVAILABLE;
+      result = CURLE_NO_CONNECTION_AVAILABLE;
+      goto done;
     }
     else {
       /*
@@ -5439,7 +5441,7 @@ static CURLcode create_conn(struct SessionHandle *data,
    */
   result = setup_range(data);
   if(result)
-    return result;
+    goto done;
 
   /* Continue connectdata initialization here. */
 
@@ -5457,6 +5459,12 @@ static CURLcode create_conn(struct SessionHandle *data,
    *************************************************************/
   result = resolve_server(data, conn, async);
 
+  done:
+
+  Curl_safefree(proxy);
+  Curl_safefree(options);
+  Curl_safefree(passwd);
+  Curl_safefree(user);
   return result;
 }
 
-- 
1.8.4.rc2


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to