From: Douglas Bagnall Date: Thu, 16 Mar 2023 02:46:08 +0000 (+1300) Subject: libcli/sec/sddl decode: allow hex numbers in SIDs X-Git-Tag: talloc-2.4.1~835 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b621c59f64cb85877e4fd116f31e5556f146d88e;p=thirdparty%2Fsamba.git 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 Reviewed-by: Andrew Bartlett --- 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; } diff --git a/selftest/knownfail.d/sid-strings b/selftest/knownfail.d/sid-strings index 4fc0e4127b9..9acc2b51a5a 100644 --- a/selftest/knownfail.d/sid-strings +++ b/selftest/knownfail.d/sid-strings @@ -68,14 +68,8 @@ ^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_S-000000000001-5-20-243.ad_dc ^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_S-000000001-5-32-579.ad_dc ^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_S-1-0.ad_dc -^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_S-1-0x05-32-579.ad_dc -^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_S-1-0x5-0x20-0x243.ad_dc -^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_S-1-0x50000000-32-579.ad_dc -^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_S-1-0x500000000-32-579.ad_dc -^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_S-1-0xABcDef123-0xABCDef-579.ad_dc ^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_S-1-22.ad_dc ^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_S-1-281474976710656-579.ad_dc -^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_S-1-5-0x20-579.ad_dc ^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_S-1-5-3.2-579.ad_dc ^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_S-1-5-32--579.ad_dc ^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_S-1-5-32-.579.ad_dc @@ -84,12 +78,6 @@ ^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_internal_S-000000000001-5-20-243.ad_dc ^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_internal_S-000000001-5-32-579.ad_dc ^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_internal_S-1-0.ad_dc -^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_internal_S-1-0x05-32-579.ad_dc -^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_internal_S-1-0x5-0x20-0x243.ad_dc -^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_internal_S-1-0x50000000-32-579.ad_dc -^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_internal_S-1-0x500000000-32-579.ad_dc -^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_internal_S-1-0xABcDef123-0xABCDef-579.ad_dc ^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_internal_S-1-22.ad_dc -^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_internal_S-1-5-0x20-579.ad_dc ^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_internal_s-1-5-32-579.ad_dc ^samba.tests.sid_strings.+.SidStringsThatStartWithS.test_sid_string_s-1-5-32-579.ad_dc