]> git.ipfire.org Git - thirdparty/samba.git/commitdiff
auth: Cleanup exit code paths in kerberos_decode_pac().
authorJeremy Allison <jra@samba.org>
Fri, 17 Jan 2025 00:12:31 +0000 (16:12 -0800)
committerJule Anger <janger@samba.org>
Mon, 3 Feb 2025 14:53:10 +0000 (14:53 +0000)
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 <jra@samba.org>
Reviewed-by: Jennifer Sutton <jennifersutton@catalyst.net.nz>
Reviewed-by: Christian Ambach <ambi@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
Autobuild-User(master): Günther Deschner <gd@samba.org>
Autobuild-Date(master): Fri Jan 17 12:01:47 UTC 2025 on atb-devel-224

(cherry picked from commit f9eb0b248da0689c82656f3e482161c45749afb6)

auth/kerberos/kerberos_pac.c

index 1f7d3e7ef262556a686b909890702c98f0929777..4c61cfe838ffccb218f4499563c46645ada3ad9a 100644 (file)
@@ -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,