mirror of
https://github.com/curl/curl.git
synced 2026-07-02 12:37:16 +03:00
socks_sspi: Fix some memory cleanup calls
- Ensure memory allocated by malloc() is freed by free(). Prior to this change SSPI's FreeContextBuffer() was sometimes used to free malloc'd memory. I can only assume the reason we have no crash reports about this is because the underlying heap free is probably the same for both. Reported-by: Joshua Rogers Fixes https://github.com/curl/curl/issues/18587 Closes https://github.com/curl/curl/pull/18594
This commit is contained in:
Jay Satiro
parent
a80abc45a5
commit
f8175b1536
1 changed files with 55 additions and 47 deletions
102
lib/socks_sspi.c
102
lib/socks_sspi.c
|
|
@ -90,6 +90,8 @@ CURLcode Curl_SOCKS5_gssapi_negotiate(struct Curl_cfilter *cf,
|
|||
unsigned char socksreq[4]; /* room for GSS-API exchange header only */
|
||||
const char *service = data->set.str[STRING_PROXY_SERVICE_NAME] ?
|
||||
data->set.str[STRING_PROXY_SERVICE_NAME] : "rcmd";
|
||||
char *etbuf;
|
||||
size_t etbuf_size;
|
||||
|
||||
/* GSS-API request looks like
|
||||
* +----+------+-----+----------------+
|
||||
|
|
@ -131,11 +133,14 @@ CURLcode Curl_SOCKS5_gssapi_negotiate(struct Curl_cfilter *cf,
|
|||
wrap_desc.pBuffers = sspi_w_token;
|
||||
wrap_desc.ulVersion = SECBUFFER_VERSION;
|
||||
|
||||
cred_handle.dwLower = 0;
|
||||
cred_handle.dwUpper = 0;
|
||||
memset(&cred_handle, 0, sizeof(cred_handle));
|
||||
memset(&sspi_context, 0, sizeof(sspi_context));
|
||||
|
||||
names.sUserName = NULL;
|
||||
|
||||
etbuf = NULL;
|
||||
etbuf_size = 0;
|
||||
|
||||
status =
|
||||
Curl_pSecFn->AcquireCredentialsHandle(NULL,
|
||||
(TCHAR *)CURL_UNCONST(TEXT("Kerberos")),
|
||||
|
|
@ -178,11 +183,8 @@ CURLcode Curl_SOCKS5_gssapi_negotiate(struct Curl_cfilter *cf,
|
|||
|
||||
curlx_unicodefree(sname);
|
||||
|
||||
if(sspi_recv_token.pvBuffer) {
|
||||
Curl_pSecFn->FreeContextBuffer(sspi_recv_token.pvBuffer);
|
||||
sspi_recv_token.pvBuffer = NULL;
|
||||
sspi_recv_token.cbBuffer = 0;
|
||||
}
|
||||
Curl_safefree(sspi_recv_token.pvBuffer);
|
||||
sspi_recv_token.cbBuffer = 0;
|
||||
|
||||
if(check_sspi_err(data, status, "InitializeSecurityContext")) {
|
||||
failf(data, "Failed to initialise security context.");
|
||||
|
|
@ -220,10 +222,7 @@ CURLcode Curl_SOCKS5_gssapi_negotiate(struct Curl_cfilter *cf,
|
|||
}
|
||||
sspi_send_token.cbBuffer = 0;
|
||||
|
||||
if(sspi_recv_token.pvBuffer) {
|
||||
Curl_pSecFn->FreeContextBuffer(sspi_recv_token.pvBuffer);
|
||||
sspi_recv_token.pvBuffer = NULL;
|
||||
}
|
||||
Curl_safefree(sspi_recv_token.pvBuffer);
|
||||
sspi_recv_token.cbBuffer = 0;
|
||||
|
||||
if(status != SEC_I_CONTINUE_NEEDED)
|
||||
|
|
@ -289,7 +288,6 @@ CURLcode Curl_SOCKS5_gssapi_negotiate(struct Curl_cfilter *cf,
|
|||
status = Curl_pSecFn->QueryCredentialsAttributes(&cred_handle,
|
||||
SECPKG_CRED_ATTR_NAMES,
|
||||
&names);
|
||||
Curl_pSecFn->FreeCredentialsHandle(&cred_handle);
|
||||
if(check_sspi_err(data, status, "QueryCredentialAttributes")) {
|
||||
failf(data, "Failed to determine username.");
|
||||
result = CURLE_COULDNT_CONNECT;
|
||||
|
|
@ -303,6 +301,7 @@ CURLcode Curl_SOCKS5_gssapi_negotiate(struct Curl_cfilter *cf,
|
|||
curlx_unicodefree(user_utf8);
|
||||
#endif
|
||||
Curl_pSecFn->FreeContextBuffer(names.sUserName);
|
||||
names.sUserName = NULL;
|
||||
}
|
||||
|
||||
/* Do encryption */
|
||||
|
|
@ -399,35 +398,30 @@ CURLcode Curl_SOCKS5_gssapi_negotiate(struct Curl_cfilter *cf,
|
|||
result = CURLE_COULDNT_CONNECT;
|
||||
goto error;
|
||||
}
|
||||
sspi_send_token.cbBuffer = sspi_w_token[0].cbBuffer +
|
||||
sspi_w_token[1].cbBuffer +
|
||||
sspi_w_token[2].cbBuffer;
|
||||
sspi_send_token.pvBuffer = malloc(sspi_send_token.cbBuffer);
|
||||
if(!sspi_send_token.pvBuffer) {
|
||||
|
||||
etbuf_size = sspi_w_token[0].cbBuffer +
|
||||
sspi_w_token[1].cbBuffer +
|
||||
sspi_w_token[2].cbBuffer;
|
||||
etbuf = malloc(etbuf_size);
|
||||
if(!etbuf) {
|
||||
result = CURLE_OUT_OF_MEMORY;
|
||||
goto error;
|
||||
}
|
||||
|
||||
memcpy(sspi_send_token.pvBuffer, sspi_w_token[0].pvBuffer,
|
||||
sspi_w_token[0].cbBuffer);
|
||||
memcpy((PUCHAR) sspi_send_token.pvBuffer +(int)sspi_w_token[0].cbBuffer,
|
||||
memcpy(etbuf, sspi_w_token[0].pvBuffer, sspi_w_token[0].cbBuffer);
|
||||
memcpy(etbuf + sspi_w_token[0].cbBuffer,
|
||||
sspi_w_token[1].pvBuffer, sspi_w_token[1].cbBuffer);
|
||||
memcpy((PUCHAR) sspi_send_token.pvBuffer +
|
||||
sspi_w_token[0].cbBuffer +
|
||||
sspi_w_token[1].cbBuffer,
|
||||
memcpy(etbuf + sspi_w_token[0].cbBuffer + sspi_w_token[1].cbBuffer,
|
||||
sspi_w_token[2].pvBuffer, sspi_w_token[2].cbBuffer);
|
||||
|
||||
Curl_pSecFn->FreeContextBuffer(sspi_w_token[0].pvBuffer);
|
||||
sspi_w_token[0].pvBuffer = NULL;
|
||||
Curl_safefree(sspi_w_token[0].pvBuffer);
|
||||
sspi_w_token[0].cbBuffer = 0;
|
||||
Curl_pSecFn->FreeContextBuffer(sspi_w_token[1].pvBuffer);
|
||||
sspi_w_token[1].pvBuffer = NULL;
|
||||
Curl_safefree(sspi_w_token[1].pvBuffer);
|
||||
sspi_w_token[1].cbBuffer = 0;
|
||||
Curl_pSecFn->FreeContextBuffer(sspi_w_token[2].pvBuffer);
|
||||
sspi_w_token[2].pvBuffer = NULL;
|
||||
Curl_safefree(sspi_w_token[2].pvBuffer);
|
||||
sspi_w_token[2].cbBuffer = 0;
|
||||
|
||||
us_length = htons((unsigned short)sspi_send_token.cbBuffer);
|
||||
us_length = htons((unsigned short)etbuf_size);
|
||||
memcpy(socksreq + 2, &us_length, sizeof(short));
|
||||
}
|
||||
|
||||
|
|
@ -450,16 +444,14 @@ CURLcode Curl_SOCKS5_gssapi_negotiate(struct Curl_cfilter *cf,
|
|||
}
|
||||
}
|
||||
else {
|
||||
code = Curl_conn_cf_send(cf->next, data,
|
||||
(char *)sspi_send_token.pvBuffer,
|
||||
sspi_send_token.cbBuffer, FALSE, &written);
|
||||
if(code || (sspi_send_token.cbBuffer != (size_t)written)) {
|
||||
code = Curl_conn_cf_send(cf->next, data, etbuf, etbuf_size,
|
||||
FALSE, &written);
|
||||
if(code || (etbuf_size != written)) {
|
||||
failf(data, "Failed to send SSPI encryption type.");
|
||||
result = CURLE_COULDNT_CONNECT;
|
||||
goto error;
|
||||
}
|
||||
if(sspi_send_token.pvBuffer)
|
||||
Curl_pSecFn->FreeContextBuffer(sspi_send_token.pvBuffer);
|
||||
Curl_safefree(etbuf);
|
||||
}
|
||||
|
||||
result = Curl_blockread_all(cf, data, (char *)socksreq, 4, &actualread);
|
||||
|
|
@ -514,8 +506,16 @@ CURLcode Curl_SOCKS5_gssapi_negotiate(struct Curl_cfilter *cf,
|
|||
status = Curl_pSecFn->DecryptMessage(&sspi_context, &wrap_desc,
|
||||
0, &qop);
|
||||
|
||||
/* since sspi_w_token[1].pvBuffer is allocated by the SSPI in this case, it
|
||||
must be freed in this block using FreeContextBuffer() instead of
|
||||
potentially in error cleanup using free(). */
|
||||
|
||||
if(check_sspi_err(data, status, "DecryptMessage")) {
|
||||
failf(data, "Failed to query security context attributes.");
|
||||
if(sspi_w_token[1].pvBuffer) {
|
||||
Curl_pSecFn->FreeContextBuffer(sspi_w_token[1].pvBuffer);
|
||||
sspi_w_token[1].pvBuffer = NULL;
|
||||
}
|
||||
result = CURLE_COULDNT_CONNECT;
|
||||
goto error;
|
||||
}
|
||||
|
|
@ -523,13 +523,20 @@ CURLcode Curl_SOCKS5_gssapi_negotiate(struct Curl_cfilter *cf,
|
|||
if(sspi_w_token[1].cbBuffer != 1) {
|
||||
failf(data, "Invalid SSPI encryption response length (%lu).",
|
||||
(unsigned long)sspi_w_token[1].cbBuffer);
|
||||
if(sspi_w_token[1].pvBuffer) {
|
||||
Curl_pSecFn->FreeContextBuffer(sspi_w_token[1].pvBuffer);
|
||||
sspi_w_token[1].pvBuffer = NULL;
|
||||
}
|
||||
result = CURLE_COULDNT_CONNECT;
|
||||
goto error;
|
||||
}
|
||||
|
||||
memcpy(socksreq, sspi_w_token[1].pvBuffer, sspi_w_token[1].cbBuffer);
|
||||
Curl_pSecFn->FreeContextBuffer(sspi_w_token[0].pvBuffer);
|
||||
Curl_safefree(sspi_w_token[0].pvBuffer);
|
||||
sspi_w_token[0].cbBuffer = 0;
|
||||
Curl_pSecFn->FreeContextBuffer(sspi_w_token[1].pvBuffer);
|
||||
sspi_w_token[1].pvBuffer = NULL;
|
||||
sspi_w_token[1].cbBuffer = 0;
|
||||
}
|
||||
else {
|
||||
if(sspi_w_token[0].cbBuffer != 1) {
|
||||
|
|
@ -539,7 +546,8 @@ CURLcode Curl_SOCKS5_gssapi_negotiate(struct Curl_cfilter *cf,
|
|||
goto error;
|
||||
}
|
||||
memcpy(socksreq, sspi_w_token[0].pvBuffer, sspi_w_token[0].cbBuffer);
|
||||
Curl_pSecFn->FreeContextBuffer(sspi_w_token[0].pvBuffer);
|
||||
Curl_safefree(sspi_w_token[0].pvBuffer);
|
||||
sspi_w_token[0].cbBuffer = 0;
|
||||
}
|
||||
(void)curlx_nonblock(sock, TRUE);
|
||||
|
||||
|
|
@ -557,24 +565,24 @@ CURLcode Curl_SOCKS5_gssapi_negotiate(struct Curl_cfilter *cf,
|
|||
conn->socks5_sspi_context = sspi_context;
|
||||
}
|
||||
*/
|
||||
|
||||
Curl_pSecFn->DeleteSecurityContext(&sspi_context);
|
||||
Curl_pSecFn->FreeCredentialsHandle(&cred_handle);
|
||||
return CURLE_OK;
|
||||
error:
|
||||
(void)curlx_nonblock(sock, TRUE);
|
||||
free(service_name);
|
||||
Curl_pSecFn->FreeCredentialsHandle(&cred_handle);
|
||||
Curl_pSecFn->DeleteSecurityContext(&sspi_context);
|
||||
if(sspi_recv_token.pvBuffer)
|
||||
Curl_pSecFn->FreeContextBuffer(sspi_recv_token.pvBuffer);
|
||||
Curl_pSecFn->FreeCredentialsHandle(&cred_handle);
|
||||
free(sspi_recv_token.pvBuffer);
|
||||
if(sspi_send_token.pvBuffer)
|
||||
Curl_pSecFn->FreeContextBuffer(sspi_send_token.pvBuffer);
|
||||
if(names.sUserName)
|
||||
Curl_pSecFn->FreeContextBuffer(names.sUserName);
|
||||
if(sspi_w_token[0].pvBuffer)
|
||||
Curl_pSecFn->FreeContextBuffer(sspi_w_token[0].pvBuffer);
|
||||
if(sspi_w_token[1].pvBuffer)
|
||||
Curl_pSecFn->FreeContextBuffer(sspi_w_token[1].pvBuffer);
|
||||
if(sspi_w_token[2].pvBuffer)
|
||||
Curl_pSecFn->FreeContextBuffer(sspi_w_token[2].pvBuffer);
|
||||
free(sspi_w_token[0].pvBuffer);
|
||||
free(sspi_w_token[1].pvBuffer);
|
||||
free(sspi_w_token[2].pvBuffer);
|
||||
free(etbuf);
|
||||
return result;
|
||||
}
|
||||
#endif
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue