moparisthebest/curl
1
0
Fork 0
mirror of https://github.com/moparisthebest/curl synced 2024-11-17 15:05:02 -05:00
Code Issues Releases Wiki Activity

smb: fix memory leak on early failure

Browse Source 
... by making sure connection related data (->share) is stored in the
connection and not in the easy handle.

Detected by OSS-fuzz
Bug: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9369
Fixes #2769
Closes #2810
This commit is contained in:
Daniel Stenberg 2018-07-29 17:58:10 +02:00
parent fe60cbfbbf
commit 09e401e01b
No known key found for this signature in database
GPG Key ID: 5CC908FDB71E12C2
2 changed files with 37 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
lib/smb.c
View File

@ -59,6 +59,7 @@
static CURLcode smb_setup_connection(struct connectdata *conn); static CURLcode smb_setup_connection(struct connectdata *conn);
static CURLcode smb_connect(struct connectdata *conn, bool *done); static CURLcode smb_connect(struct connectdata *conn, bool *done);
static CURLcode smb_connection_state(struct connectdata *conn, bool *done); static CURLcode smb_connection_state(struct connectdata *conn, bool *done);
static CURLcode smb_do(struct connectdata *conn, bool *done);
static CURLcode smb_request_state(struct connectdata *conn, bool *done); static CURLcode smb_request_state(struct connectdata *conn, bool *done);
static CURLcode smb_done(struct connectdata *conn, CURLcode status, static CURLcode smb_done(struct connectdata *conn, CURLcode status,
bool premature); bool premature);
@ -73,7 +74,7 @@ static CURLcode smb_parse_url_path(struct connectdata *conn);
const struct Curl_handler Curl_handler_smb = { const struct Curl_handler Curl_handler_smb = {
"SMB", /* scheme */ "SMB", /* scheme */
smb_setup_connection, /* setup_connection */ smb_setup_connection, /* setup_connection */
ZERO_NULL, /* do_it */ smb_do, /* do_it */
smb_done, /* done */ smb_done, /* done */
ZERO_NULL, /* do_more */ ZERO_NULL, /* do_more */
smb_connect, /* connect_it */ smb_connect, /* connect_it */
@ -98,7 +99,7 @@ const struct Curl_handler Curl_handler_smb = {
const struct Curl_handler Curl_handler_smbs = { const struct Curl_handler Curl_handler_smbs = {
"SMBS", /* scheme */ "SMBS", /* scheme */
smb_setup_connection, /* setup_connection */ smb_setup_connection, /* setup_connection */
ZERO_NULL, /* do_it */ smb_do, /* do_it */
smb_done, /* done */ smb_done, /* done */
ZERO_NULL, /* do_more */ ZERO_NULL, /* do_more */
smb_connect, /* connect_it */ smb_connect, /* connect_it */
@ -173,7 +174,6 @@ enum smb_req_state {
/* SMB request data */ /* SMB request data */
struct smb_request { struct smb_request {
enum smb_req_state state; enum smb_req_state state;
char *share;
char *path; char *path;
unsigned short tid; /* Even if we connect to the same tree as another */ unsigned short tid; /* Even if we connect to the same tree as another */
unsigned short fid; /* request, the tid will be different */ unsigned short fid; /* request, the tid will be different */
@ -182,7 +182,7 @@ struct smb_request {
static void conn_state(struct connectdata *conn, enum smb_conn_state newstate) static void conn_state(struct connectdata *conn, enum smb_conn_state newstate)
{ {
struct smb_conn *smb = &conn->proto.smbc; struct smb_conn *smbc = &conn->proto.smbc;
#if defined(DEBUGBUILD) && !defined(CURL_DISABLE_VERBOSE_STRINGS) #if defined(DEBUGBUILD) && !defined(CURL_DISABLE_VERBOSE_STRINGS)
/* For debug purposes */ /* For debug purposes */
static const char * const names[] = { static const char * const names[] = {
@ -194,12 +194,12 @@ static void conn_state(struct connectdata *conn, enum smb_conn_state newstate)
/* LAST */ /* LAST */
}; };
if(smb->state != newstate) if(smbc->state != newstate)
infof(conn->data, "SMB conn %p state change from %s to %s\n", infof(conn->data, "SMB conn %p state change from %s to %s\n",
(void *)smb, names[smb->state], names[newstate]); (void *)smbc, names[smbc->state], names[newstate]);
#endif #endif
smb->state = newstate; smbc->state = newstate;
} }
static void request_state(struct connectdata *conn, static void request_state(struct connectdata *conn,
@ -228,6 +228,8 @@ static void request_state(struct connectdata *conn,
req->state = newstate; req->state = newstate;
} }
/* this should setup things in the connection, not in the easy
handle */
static CURLcode smb_setup_connection(struct connectdata *conn) static CURLcode smb_setup_connection(struct connectdata *conn)
{ {
struct smb_request *req; struct smb_request *req;
@ -253,7 +255,6 @@ static CURLcode smb_connect(struct connectdata *conn, bool *done)
return CURLE_LOGIN_DENIED; return CURLE_LOGIN_DENIED;
/* Initialize the connection state */ /* Initialize the connection state */
memset(smbc, 0, sizeof(*smbc));
smbc->state = SMB_CONNECTING; smbc->state = SMB_CONNECTING;
smbc->recv_buf = malloc(MAX_MESSAGE_SIZE); smbc->recv_buf = malloc(MAX_MESSAGE_SIZE);
if(!smbc->recv_buf) if(!smbc->recv_buf)
@ -475,11 +476,11 @@ static CURLcode smb_send_setup(struct connectdata *conn)
static CURLcode smb_send_tree_connect(struct connectdata *conn) static CURLcode smb_send_tree_connect(struct connectdata *conn)
{ {
struct smb_request *req = conn->data->req.protop;
struct smb_tree_connect msg; struct smb_tree_connect msg;
struct smb_conn *smbc = &conn->proto.smbc;
char *p = msg.bytes; char *p = msg.bytes;
size_t byte_count = strlen(conn->host.name) + strlen(req->share); size_t byte_count = strlen(conn->host.name) + strlen(smbc->share);
byte_count += strlen(SERVICENAME) + 5; /* 2 nulls and 3 backslashes */ byte_count += strlen(SERVICENAME) + 5; /* 2 nulls and 3 backslashes */
if(byte_count > sizeof(msg.bytes)) if(byte_count > sizeof(msg.bytes))
return CURLE_FILESIZE_EXCEEDED; return CURLE_FILESIZE_EXCEEDED;
@ -491,7 +492,7 @@ static CURLcode smb_send_tree_connect(struct connectdata *conn)
MSGCAT("\\\\"); MSGCAT("\\\\");
MSGCAT(conn->host.name); MSGCAT(conn->host.name);
MSGCAT("\\"); MSGCAT("\\");
MSGCATNULL(req->share); MSGCATNULL(smbc->share);
MSGCATNULL(SERVICENAME); /* Match any type of service */ MSGCATNULL(SERVICENAME); /* Match any type of service */
byte_count = p - msg.bytes; byte_count = p - msg.bytes;
msg.byte_count = smb_swap16((unsigned short)byte_count); msg.byte_count = smb_swap16((unsigned short)byte_count);
@ -910,31 +911,18 @@ static CURLcode smb_request_state(struct connectdata *conn, bool *done)
static CURLcode smb_done(struct connectdata *conn, CURLcode status, static CURLcode smb_done(struct connectdata *conn, CURLcode status,
bool premature) bool premature)
{ {
struct smb_request *req = conn->data->req.protop;
(void) premature; (void) premature;
Curl_safefree(req->share);
Curl_safefree(conn->data->req.protop); Curl_safefree(conn->data->req.protop);
return status; return status;
} }
static CURLcode smb_disconnect(struct connectdata *conn, bool dead) static CURLcode smb_disconnect(struct connectdata *conn, bool dead)
{ {
struct smb_conn *smbc = &conn->proto.smbc; struct smb_conn *smbc = &conn->proto.smbc;
struct smb_request *req = conn->data->req.protop;
(void) dead; (void) dead;
Curl_safefree(smbc->share);
Curl_safefree(smbc->domain); Curl_safefree(smbc->domain);
Curl_safefree(smbc->recv_buf); Curl_safefree(smbc->recv_buf);
/* smb_done is not always called, so cleanup the request */
if(req) {
Curl_safefree(req->share);
}
return CURLE_OK; return CURLE_OK;
} }
@ -948,11 +936,27 @@ static int smb_getsock(struct connectdata *conn, curl_socket_t *socks,
return GETSOCK_READSOCK(0) | GETSOCK_WRITESOCK(0); return GETSOCK_READSOCK(0) | GETSOCK_WRITESOCK(0);
} }
static CURLcode smb_do(struct connectdata *conn, bool *done)
{
struct smb_conn *smbc = &conn->proto.smbc;
struct smb_request *req = conn->data->req.protop;
if(smbc->share) {
req->path = strchr(smbc->share, '\0');
if(req->path) {
req->path++;
*done = TRUE;
return CURLE_OK;
}
}
return CURLE_URL_MALFORMAT;
}
static CURLcode smb_parse_url_path(struct connectdata *conn) static CURLcode smb_parse_url_path(struct connectdata *conn)
{ {
CURLcode result = CURLE_OK; CURLcode result = CURLE_OK;
struct Curl_easy *data = conn->data; struct Curl_easy *data = conn->data;
struct smb_request *req = data->req.protop; struct smb_conn *smbc = &conn->proto.smbc;
char *path; char *path;
char *slash; char *slash;
@ -962,30 +966,29 @@ static CURLcode smb_parse_url_path(struct connectdata *conn)
return result; return result;
/* Parse the path for the share */ /* Parse the path for the share */
req->share = strdup((*path == '/' || *path == '\\') ? path + 1 : path); smbc->share = strdup((*path == '/' || *path == '\\') ? path + 1 : path);
free(path); free(path);
if(!req->share) if(!smbc->share)
return CURLE_OUT_OF_MEMORY; return CURLE_OUT_OF_MEMORY;
slash = strchr(req->share, '/'); slash = strchr(smbc->share, '/');
if(!slash) if(!slash)
slash = strchr(req->share, '\\'); slash = strchr(smbc->share, '\\');
/* The share must be present */ /* The share must be present */
if(!slash) { if(!slash) {
Curl_safefree(req->share); Curl_safefree(smbc->share);
return CURLE_URL_MALFORMAT; return CURLE_URL_MALFORMAT;
} }
/* Parse the path for the file path converting any forward slashes into /* Parse the path for the file path converting any forward slashes into
backslashes */ backslashes */
*slash++ = 0; *slash++ = 0;
req->path = slash;
for(; *slash; slash++) { for(; *slash; slash++) {
if(*slash == '/') if(*slash == '/')
*slash = '\\'; *slash = '\\';
} }
return CURLE_OK; return CURLE_OK;
} }

1
lib/smb.h
View File

@ -35,6 +35,7 @@ struct smb_conn {
enum smb_conn_state state; enum smb_conn_state state;
char *user; char *user;
char *domain; char *domain;
char *share;
unsigned char challenge[8]; unsigned char challenge[8];
unsigned int session_key; unsigned int session_key;
unsigned short uid; unsigned short uid;