← Commit list

f8175b1536

Jay Satiro
Jay Satiro
2025-09-18
FAILED Duration: 240s

Subject

socks_sspi: Fix some memory cleanup calls

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

lib/socks_sspi.c +55 -47
lib/socks_sspi.c+55 -47
@@ -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