libcli/sec/sddl decode: allow hex numbers in SIDs

These occur canonically when the indentifier authority is > 2^32, but
also are accepted by Windows for any number.

There is a tricky case with an "O:" or "G:" SID that is immediately
followed by a "D:" dacl, because the "D" looks like a hex digit. When
we detect this we need to subtract one from the length.

We also need to do look out for trailing garbage. This was not an
issue before because any string caught by the strspn(...,
"-0123456789") would be either rejected or fully comsumed by
dom_sid_parse_talloc(), but with hex digits, a string like
"S-1-1-2x0xabcxxx-X" would be successfully parsed as "S-1-1-2", and
the "x0xabcxxx-X" would be skipped over. That's why we switch to using
dom_sid_parse_endp(), so we can compare the consumed length to the
expected length.

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

1 files changed, 39 insertions, 8 deletions

diff --git a/libcli/security/sddl.c b/libcli/security/sddl.c
index 3b2cdfae17a..6a9e6bdb22c 100644
--- a/libcli/security/sddl.c
+++ b/libcli/security/sddl.c
@@ -191,16 +191,47 @@ static struct dom_sid *sddl_decode_sid(TALLOC_CTX *mem_ctx, const char **sddlp,
 
 	/* see if its in the numeric format */
 	if (strncmp(sddl, "S-", 2) == 0) {
-		struct dom_sid *sid;
-		char *sid_str;
-		size_t len = strspn(sddl+2, "-0123456789");
-		sid_str = talloc_strndup(mem_ctx, sddl, len+2);
-		if (!sid_str) {
+		struct dom_sid *sid = NULL;
+		char *sid_str = NULL;
+                const char *end = NULL;
+                bool ok;
+		size_t len = strspn(sddl + 2, "-0123456789ABCDEFabcdefxX") + 2;
+		if (len < 5) { /* S-1-x */
 			return NULL;
 		}
-		(*sddlp) += len+2;
-		sid = dom_sid_parse_talloc(mem_ctx, sid_str);
-		talloc_free(sid_str);
+		if (sddl[len - 1] == 'D' && sddl[len] == ':') {
+			/*
+			 * we have run into the "D:" dacl marker, mistaking it
+			 * for a hex digit. There is no other way for this
+			 * pair to occur at the end of a SID in SDDL.
+			 */
+			len--;
+		}
+
+		sid_str = talloc_strndup(mem_ctx, sddl, len);
+		if (sid_str == NULL) {
+			return NULL;
+		}
+                sid = talloc(mem_ctx, struct dom_sid);
+                if (sid == NULL) {
+			TALLOC_FREE(sid_str);
+                        return NULL;
+                };
+                ok = dom_sid_parse_endp(sid_str, sid, &end);
+                if (!ok) {
+			DBG_WARNING("could not parse SID '%s'\n", sid_str);
+			TALLOC_FREE(sid_str);
+                        TALLOC_FREE(sid);
+                        return NULL;
+                }
+		if (end - sid_str != len) {
+			DBG_WARNING("trailing junk after SID '%s'\n", sid_str);
+			TALLOC_FREE(sid_str);
+                        TALLOC_FREE(sid);
+                        return NULL;
+		}
+		TALLOC_FREE(sid_str);
+		(*sddlp) += len;
 		return sid;
 	}