From: Pavel Filipenský <pfilipensky@samba.org>
Date: Wed, 26 Jul 2023 14:28:36 +0000 (+0200)
Subject: auth:kerberos: Fix resource leak in parse_principal()
X-Git-Tag: tevent-0.16.0~1306
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dfc26dc494eb9d80fe5b19b0ed41cedd0e187dbd;p=thirdparty%2Fsamba.git

auth:kerberos: Fix resource leak in parse_principal()

Reported by Red Hat internal covscan
leaked_storage: Variable "princ" going out of scope leaks the storage it points to.

Signed-off-by: Pavel Filipenský <pfilipensky@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
---

diff --git a/source4/auth/kerberos/kerberos_util.c b/source4/auth/kerberos/kerberos_util.c
index ec164aaedad..432266aab91 100644
--- a/source4/auth/kerberos/kerberos_util.c
+++ b/source4/auth/kerberos/kerberos_util.c
@@ -56,6 +56,24 @@ static krb5_error_code parse_principal(TALLOC_CTX *parent_ctx,
 		 return 0;
 	}
 
+	/*
+	 * Start with talloc(), talloc_reference() and only then call
+	 * krb5_parse_name(). If any of them fails, the cleanup code is simpler.
+	 */
+	mem_ctx = talloc(parent_ctx, struct principal_container);
+	if (!mem_ctx) {
+		(*error_string) = error_message(ENOMEM);
+		return ENOMEM;
+	}
+
+	mem_ctx->smb_krb5_context = talloc_reference(mem_ctx,
+						     smb_krb5_context);
+	if (mem_ctx->smb_krb5_context == NULL) {
+		(*error_string) = error_message(ENOMEM);
+		talloc_free(mem_ctx);
+		return ENOMEM;
+	}
+
 	ret = krb5_parse_name(smb_krb5_context->krb5_context,
 			      princ_string, princ);
 
@@ -63,19 +81,12 @@ static krb5_error_code parse_principal(TALLOC_CTX *parent_ctx,
 		(*error_string) = smb_get_krb5_error_message(
 						smb_krb5_context->krb5_context,
 						ret, parent_ctx);
+		talloc_free(mem_ctx);
 		return ret;
 	}
 
-	mem_ctx = talloc(parent_ctx, struct principal_container);
-	if (!mem_ctx) {
-		(*error_string) = error_message(ENOMEM);
-		return ENOMEM;
-	}
-
 	/* This song-and-dance effectivly puts the principal
 	 * into talloc, so we can't loose it. */
-	mem_ctx->smb_krb5_context = talloc_reference(mem_ctx,
-						     smb_krb5_context);
 	mem_ctx->principal = *princ;
 	talloc_set_destructor(mem_ctx, free_principal);
 	return 0;