Bug#719856: curl: silently truncates long passwords
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 | tee out
pid=$!
# Ask curl to pass a long password to that server.
curl --user me:$s http://localhost:
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
Bug#719856: curl: silently truncates long passwords
tags 719856 - patch
quit
Jonathan Nieder wrote:
How about something like
this patch?
Here's a more complete patch against Daniel's master. It doesn't
pass the test suite yet.
If this makes sense, I can split it into smaller pieces:
1. use the goto out for exception handling in create_conn
2. allocate user, password, and options on the heap instead of the
stack
3. handle long usernames and passwords in netrc
4. handle long usernames, passwords, and options from curl_easy_setopt
(the title feature!)
5. deal with exceptional cases first and use the goto out idiom
in parse_url_login
6. handle long usernames and passwords from URL.
That would make it easier to find out which change is breaking tests
and to review the changes.
Thoughts of all kinds welcome, as always.
---
lib/netrc.c | 20 ++--
lib/netrc.h | 14 +--
lib/url.c | 264 --
tests/unit/unit1304.c | 53 +-
4 files changed, 196 insertions(+), 155 deletions(-)
diff --git a/lib/netrc.c b/lib/netrc.c
index 2c5942af..f51fdf34 100644
--- a/lib/netrc.c
+++ b/lib/netrc.c
@@ -52,13 +52,13 @@ enum host_lookup_state {
* @unittest: 1304
*/
int Curl_parsenetrc(const char *host,
-char *login,
-char *password,
+char **loginp,
+char **passwordp,
char *netrcfile)
{
FILE *file;
int retcode=1;
- int specific_login = (login[0] != 0);
+ int specific_login = (**loginp != 0);
char *home = NULL;
bool home_alloc = FALSE;
bool netrc_alloc = FALSE;
@@ -109,7 +109,7 @@ int Curl_parsenetrc(const char *host,
tok=strtok_r(netrcbuffer, \t\n, tok_buf);
while(!done tok) {
-if(login[0] password[0]) {
+if(**loginp **passwordp) {
done=TRUE;
break;
}
@@ -138,16 +138,22 @@ int Curl_parsenetrc(const char *host,
/* we are now parsing sub-keywords concerning our host */
if(state_login) {
if(specific_login) {
- state_our_login = Curl_raw_equal(login, tok);
+ state_our_login = Curl_raw_equal(*loginp, tok);
}
else {
- strncpy(login, tok, LOGINSIZE-1);
+ free(*loginp);
+ *loginp = strdup(tok);
+ if(!*loginp)
+return -1; /* allocation failed */
}
state_login=0;
}
else if(state_password) {
if(state_our_login || !specific_login) {
- strncpy(password, tok, PASSWORDSIZE-1);
+ free(*passwordp);
+ *passwordp = strdup(tok);
+ if(!*passwordp)
+return -1; /* allocation failed */
}
state_password=0;
}
diff --git a/lib/netrc.h b/lib/netrc.h
index 4db764df..6bd9df6f 100644
--- a/lib/netrc.h
+++ b/lib/netrc.h
@@ -22,19 +22,15 @@
*
***/
-/* Make sure we have room for at least this size: */
-#define LOGINSIZE 64
-#define PASSWORDSIZE 64
-
/* returns -1 on failure, 0 if the host is found, 1 is the host isn't found */
int Curl_parsenetrc(const char *host,
-char *login,
-char *password,
+char **loginp,
+char **passwordp,
char *filename);
- /* Assume: password[0]=0, host[0] != 0.
- * If login[0] = 0, search for login and password within a machine section
+ /* Assume: (*passwordp)[0]=0, host[0] != 0.
+ * If (*loginp)[0] = 0, search for login and password within a machine
section
* in the netrc.
- * If login[0] != 0, search for password within machine and login.
+ * If (*loginp)[0] != 0, search for password within machine and login.
*/
#endif /* HEADER_CURL_NETRC_H */
diff --git a/lib/url.c b/lib/url.c
index 878f3873..fb56006e 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,
-
