From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Date: Wed, 22 Nov 2023 01:39:49 +0000 (+1300)
Subject: libcli/security: simplify wire claim conversion mem, 1/3: avoid NULL parent
X-Git-Tag: talloc-2.4.2~510
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=7656d1333459e2ebd3d9983283629609d0db6d96;p=thirdparty%2Fsamba.git

libcli/security: simplify wire claim conversion mem, 1/3: avoid NULL parent

The reason for this, apart from weighing up possible over-allocations
vs realloc costs, is in the first iteration of the loop,

       claim_values = talloc_array(claims,

would allocate onto NULL, which leaks.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---

diff --git a/libcli/security/claims-conversions.c b/libcli/security/claims-conversions.c
index 85206511883..a1e36d4724f 100644
--- a/libcli/security/claims-conversions.c
+++ b/libcli/security/claims-conversions.c
@@ -711,6 +711,7 @@ NTSTATUS token_claims_to_claims_v1(TALLOC_CTX *mem_ctx,
 	TALLOC_CTX *tmp_ctx = NULL;
 	struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *claims = NULL;
 	uint32_t n_claims = 0;
+	uint32_t expected_n_claims = 0;
 	uint32_t i;
 
 	if (out_claims == NULL) {
@@ -732,6 +733,27 @@ NTSTATUS token_claims_to_claims_v1(TALLOC_CTX *mem_ctx,
 		return NT_STATUS_NO_MEMORY;
 	}
 
+
+	/*
+	 * The outgoing number of claims is (at most) the sum of the
+	 * claims_counts of each claims_array.
+	 */
+	for (i = 0; i < claims_set->claims_array_count; ++i) {
+		uint32_t count = claims_set->claims_arrays[i].claims_count;
+		expected_n_claims += count;
+		if (expected_n_claims < count) {
+			return NT_STATUS_INVALID_PARAMETER;
+		}
+	}
+
+	claims = talloc_array(tmp_ctx,
+			      struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1,
+			      expected_n_claims);
+	if (claims == NULL) {
+		talloc_free(tmp_ctx);
+		return NT_STATUS_NO_MEMORY;
+	}
+
 	for (i = 0; i < claims_set->claims_array_count; ++i) {
 		const struct CLAIMS_ARRAY *claims_array = &claims_set->claims_arrays[i];
 		uint32_t j;
@@ -905,15 +927,6 @@ NTSTATUS token_claims_to_claims_v1(TALLOC_CTX *mem_ctx,
 				continue;
 			}
 
-			claims = talloc_realloc(tmp_ctx,
-						claims,
-						struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1,
-						++n_claims);
-			if (claims == NULL) {
-				talloc_free(tmp_ctx);
-				return NT_STATUS_NO_MEMORY;
-			}
-
 			if (claim_entry->id != NULL) {
 				name = talloc_strdup(claims, claim_entry->id);
 				if (name == NULL) {
@@ -922,13 +935,14 @@ NTSTATUS token_claims_to_claims_v1(TALLOC_CTX *mem_ctx,
 				}
 			}
 
-			claims[n_claims - 1] = (struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1) {
+			claims[n_claims] = (struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1) {
 				.name = name,
 				.value_type = value_type,
 				.flags = 0,
 				.value_count = n_values,
 				.values = claim_values,
 			};
+			n_claims++;
 		}
 	}