From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Date: Thu, 9 Nov 2023 04:56:48 +0000 (+1300)
Subject: libcli/security:sddl_decode_err_msg(): don't pretend msg is optional (CID1548624)
X-Git-Tag: talloc-2.4.2~703
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b18fe675c0d015b70cd6a1a6b8237438b345cc8a;p=thirdparty%2Fsamba.git

libcli/security:sddl_decode_err_msg(): don't pretend msg is optional (CID1548624)

Having it optionally NULL just complicates the code, and Coverity
rightly complained.

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

diff --git a/libcli/security/sddl.c b/libcli/security/sddl.c
index 97e579cfe32..f0f25b89b59 100644
--- a/libcli/security/sddl.c
+++ b/libcli/security/sddl.c
@@ -842,23 +842,23 @@ struct security_descriptor *sddl_decode_err_msg(TALLOC_CTX *mem_ctx, const char
 		.forest_sid = domain_sid,
 	};
 	const char *start = sddl;
-	struct security_descriptor *sd;
+	struct security_descriptor *sd = NULL;
+
+	if (msg == NULL || msg_offset == NULL) {
+		DBG_ERR("Programmer misbehaviour: use sddl_decode() "
+			"or provide msg pointers.\n");
+		return NULL;
+	}
+	*msg = NULL;
+	*msg_offset = 0;
+
 	sd = talloc_zero(mem_ctx, struct security_descriptor);
 	if (sd == NULL) {
-		goto failed;
+		return NULL;
 	}
 	sd->revision = SECURITY_DESCRIPTOR_REVISION_1;
 	sd->type     = SEC_DESC_SELF_RELATIVE;
 
-	if (msg != NULL) {
-		if (msg_offset == NULL) {
-			DBG_ERR("Programmer misbehaviour\n");
-			goto failed;
-		}
-		*msg = NULL;
-		*msg_offset = 0;
-	}
-
 	while (*sddl) {
 		uint32_t flags;
 		char c = sddl[0];
@@ -896,16 +896,14 @@ struct security_descriptor *sddl_decode_err_msg(TALLOC_CTX *mem_ctx, const char
 	}
 	return sd;
 failed:
-	if (msg != NULL) {
-		if (*msg != NULL) {
-			*msg = talloc_steal(mem_ctx, *msg);
-		}
-		/*
-		 * The actual message (*msg) might still be NULL, but the
-		 * offset at least provides a clue.
-		 */
-		*msg_offset += sddl - start;
+	if (*msg != NULL) {
+		*msg = talloc_steal(mem_ctx, *msg);
 	}
+	/*
+	 * The actual message (*msg) might still be NULL, but the
+	 * offset at least provides a clue.
+	 */
+	*msg_offset += sddl - start;
 	DEBUG(2,("Badly formatted SDDL '%s'\n", sddl));
 	talloc_free(sd);
 	return NULL;