The branch, master has been updated
via f9eb0b248da auth: Cleanup exit code paths in kerberos_decode_pac().
from c514ce8dcad auth: Add missing talloc_free() in error code path.
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit f9eb0b248da0689c82656f3e482161c45749afb6
Author: Jeremy Allison <[email protected]>
Date: Thu Jan 16 16:12:31 2025 -0800
auth: Cleanup exit code paths in kerberos_decode_pac().
One more memory leak missed and now fixed. tmp_ctx
must be freed once the pac data is talloc_move'd.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15782
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Jennifer Sutton <[email protected]>
Reviewed-by: Christian Ambach <[email protected]>
Reviewed-by: Guenther Deschner <[email protected]>
Autobuild-User(master): Günther Deschner <[email protected]>
Autobuild-Date(master): Fri Jan 17 12:01:47 UTC 2025 on atb-devel-224
-----------------------------------------------------------------------
Summary of changes:
auth/kerberos/kerberos_pac.c | 88 ++++++++++++++++++++++----------------------
1 file changed, 43 insertions(+), 45 deletions(-)
Changeset truncated at 500 lines:
diff --git a/auth/kerberos/kerberos_pac.c b/auth/kerberos/kerberos_pac.c
index 1f7d3e7ef26..4c61cfe838f 100644
--- a/auth/kerberos/kerberos_pac.c
+++ b/auth/kerberos/kerberos_pac.c
@@ -137,7 +137,7 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
time_t tgs_authtime,
struct PAC_DATA **pac_data_out)
{
- NTSTATUS status;
+ NTSTATUS status = NT_STATUS_NO_MEMORY;
enum ndr_err_code ndr_err;
krb5_error_code ret;
DATA_BLOB modified_pac_blob;
@@ -173,8 +173,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
kdc_sig_wipe = talloc(tmp_ctx, struct PAC_SIGNATURE_DATA);
srv_sig_wipe = talloc(tmp_ctx, struct PAC_SIGNATURE_DATA);
if (!pac_data_raw || !pac_data || !kdc_sig_wipe || !srv_sig_wipe) {
- talloc_free(tmp_ctx);
- return NT_STATUS_NO_MEMORY;
+ status = NT_STATUS_NO_MEMORY;
+ goto out;
}
ndr_err = ndr_pull_struct_blob(&pac_data_blob, pac_data, pac_data,
@@ -183,15 +183,14 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
status = ndr_map_error2ntstatus(ndr_err);
DEBUG(0,("can't parse the PAC: %s\n",
nt_errstr(status)));
- talloc_free(tmp_ctx);
- return status;
+ goto out;
}
if (pac_data->num_buffers < 4) {
/* we need logon_info, service_key and kdc_key */
DEBUG(0,("less than 4 PAC buffers\n"));
- talloc_free(tmp_ctx);
- return NT_STATUS_INVALID_PARAMETER;
+ status = NT_STATUS_INVALID_PARAMETER;
+ goto out;
}
ndr_err = ndr_pull_struct_blob(
@@ -201,15 +200,14 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
status = ndr_map_error2ntstatus(ndr_err);
DEBUG(0,("can't parse the PAC: %s\n",
nt_errstr(status)));
- talloc_free(tmp_ctx);
- return status;
+ goto out;
}
if (pac_data_raw->num_buffers < 4) {
/* we need logon_info, service_key and kdc_key */
DEBUG(0,("less than 4 PAC buffers\n"));
- talloc_free(tmp_ctx);
- return NT_STATUS_INVALID_PARAMETER;
+ status = NT_STATUS_INVALID_PARAMETER;
+ goto out;
}
if (pac_data->num_buffers != pac_data_raw->num_buffers) {
@@ -217,8 +215,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
DEBUG(0, ("misparse! PAC_DATA has %d buffers while "
"PAC_DATA_RAW has %d\n", pac_data->num_buffers,
pac_data_raw->num_buffers));
- talloc_free(tmp_ctx);
- return NT_STATUS_INVALID_PARAMETER;
+ status = NT_STATUS_INVALID_PARAMETER;
+ goto out;
}
for (i=0; i < pac_data->num_buffers; i++) {
@@ -229,8 +227,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
DEBUG(0, ("misparse! PAC_DATA buffer %d has type "
"%d while PAC_DATA_RAW has %d\n", i,
data_buf->type, raw_buf->type));
- talloc_free(tmp_ctx);
- return NT_STATUS_INVALID_PARAMETER;
+ status = NT_STATUS_INVALID_PARAMETER;
+ goto out;
}
switch (data_buf->type) {
case PAC_TYPE_LOGON_INFO:
@@ -263,26 +261,26 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
if (!logon_info) {
DEBUG(0,("PAC no logon_info\n"));
- talloc_free(tmp_ctx);
- return NT_STATUS_INVALID_PARAMETER;
+ status = NT_STATUS_INVALID_PARAMETER;
+ goto out;
}
if (!logon_name) {
DEBUG(0,("PAC no logon_name\n"));
- talloc_free(tmp_ctx);
- return NT_STATUS_INVALID_PARAMETER;
+ status = NT_STATUS_INVALID_PARAMETER;
+ goto out;
}
if (!srv_sig_ptr || !srv_sig_blob) {
DEBUG(0,("PAC no srv_key\n"));
- talloc_free(tmp_ctx);
- return NT_STATUS_INVALID_PARAMETER;
+ status = NT_STATUS_INVALID_PARAMETER;
+ goto out;
}
if (!kdc_sig_ptr || !kdc_sig_blob) {
DEBUG(0,("PAC no kdc_key\n"));
- talloc_free(tmp_ctx);
- return NT_STATUS_INVALID_PARAMETER;
+ status = NT_STATUS_INVALID_PARAMETER;
+ goto out;
}
/* Find and zero out the signatures,
@@ -297,8 +295,7 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
status = ndr_map_error2ntstatus(ndr_err);
DEBUG(0,("can't parse the KDC signature: %s\n",
nt_errstr(status)));
- talloc_free(tmp_ctx);
- return status;
+ goto out;
}
ndr_err = ndr_pull_struct_blob(
@@ -308,8 +305,7 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
status = ndr_map_error2ntstatus(ndr_err);
DEBUG(0,("can't parse the SRV signature: %s\n",
nt_errstr(status)));
- talloc_free(tmp_ctx);
- return status;
+ goto out;
}
/* Now zero the decoded structure */
@@ -326,8 +322,7 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
status = ndr_map_error2ntstatus(ndr_err);
DEBUG(0,("can't repack the KDC signature: %s\n",
nt_errstr(status)));
- talloc_free(tmp_ctx);
- return status;
+ goto out;
}
ndr_err = ndr_push_struct_blob(
srv_sig_blob, pac_data_raw, srv_sig_wipe,
@@ -336,8 +331,7 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
status = ndr_map_error2ntstatus(ndr_err);
DEBUG(0,("can't repack the SRV signature: %s\n",
nt_errstr(status)));
- talloc_free(tmp_ctx);
- return status;
+ goto out;
}
/* push out the whole structure, but now with zero'ed signatures */
@@ -348,8 +342,7 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
status = ndr_map_error2ntstatus(ndr_err);
DEBUG(0,("can't repack the RAW PAC: %s\n",
nt_errstr(status)));
- talloc_free(tmp_ctx);
- return status;
+ goto out;
}
if (service_keyblock) {
@@ -360,8 +353,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
if (ret) {
DEBUG(5, ("PAC Decode: Failed to verify the service "
"signature: %s\n", error_message(ret)));
- talloc_free(tmp_ctx);
- return NT_STATUS_ACCESS_DENIED;
+ status = NT_STATUS_ACCESS_DENIED;
+ goto out;
}
if (krbtgt_keyblock) {
@@ -371,8 +364,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
if (ret) {
DEBUG(1, ("PAC Decode: Failed to verify the KDC
signature: %s\n",
smb_get_krb5_error_message(context,
ret, tmp_ctx)));
- talloc_free(tmp_ctx);
- return NT_STATUS_ACCESS_DENIED;
+ status = NT_STATUS_ACCESS_DENIED;
+ goto out;
}
}
}
@@ -388,8 +381,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
nt_time_string(tmp_ctx,
logon_name->logon_time)));
DEBUG(2, ("PAC Decode: Ticket: %s\n",
nt_time_string(tmp_ctx,
tgs_authtime_nttime)));
- talloc_free(tmp_ctx);
- return NT_STATUS_ACCESS_DENIED;
+ status = NT_STATUS_ACCESS_DENIED;
+ goto out;
}
}
@@ -401,8 +394,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
if (ret) {
DEBUG(2, ("Could not unparse name from ticket to match
with name from PAC: [%s]:%s\n",
logon_name->account_name,
error_message(ret)));
- talloc_free(tmp_ctx);
- return NT_STATUS_INVALID_PARAMETER;
+ status = NT_STATUS_INVALID_PARAMETER;
+ goto out;
}
bool_ret = strcmp(client_principal_string,
logon_name->account_name) == 0;
@@ -413,8 +406,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
logon_name->account_name,
client_principal_string));
SAFE_FREE(client_principal_string);
- talloc_free(tmp_ctx);
- return NT_STATUS_ACCESS_DENIED;
+ status = NT_STATUS_ACCESS_DENIED;
+ goto out;
}
SAFE_FREE(client_principal_string);
@@ -435,10 +428,15 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
}
if (pac_data_out) {
- *pac_data_out = talloc_steal(mem_ctx, pac_data);
+ *pac_data_out = talloc_move(mem_ctx, &pac_data);
}
- return NT_STATUS_OK;
+ status = NT_STATUS_OK;
+
+ out:
+
+ TALLOC_FREE(tmp_ctx);
+ return status;
}
NTSTATUS kerberos_pac_logon_info(TALLOC_CTX *mem_ctx,
--
Samba Shared Repository
