From: Andrew Bartlett Date: Sat, 4 Sep 2010 04:09:17 +0000 (+1000) Subject: libcli/security Merge source3/ string_to_sid() to common code X-Git-Url: http://git.samba.org/?a=commitdiff_plain;h=51ecf796549287b7f10092778ffb52e018ae32fe;p=mat%2Fsamba.git libcli/security Merge source3/ string_to_sid() to common code The source3 code repsects the limit of a maximum of 15 subauths, while the source4 code does not, creating a security issue as we parse string-form SIDs from clients. Andrew Bartlett --- diff --git a/libcli/security/dom_sid.c b/libcli/security/dom_sid.c index ef534e3288..f044733316 100644 --- a/libcli/security/dom_sid.c +++ b/libcli/security/dom_sid.c @@ -85,60 +85,115 @@ bool dom_sid_equal(const struct dom_sid *sid1, const struct dom_sid *sid2) return dom_sid_compare(sid1, sid2) == 0; } -/* Yes, I did think about multibyte issues here, and for all I can see there's - * none of those for parsing a SID. */ -#undef strncasecmp +/***************************************************************** + Add a rid to the end of a sid +*****************************************************************/ -bool dom_sid_parse(const char *sidstr, struct dom_sid *ret) +bool sid_append_rid(struct dom_sid *sid, uint32_t rid) { - unsigned int rev, ia, num_sub_auths, i; - char *p; + if (sid->num_auths < ARRAY_SIZE(sid->sub_auths)) { + sid->sub_auths[sid->num_auths++] = rid; + return true; + } + return false; +} - if (strncasecmp(sidstr, "S-", 2)) { - return false; +/***************************************************************** + Convert a string to a SID. Returns True on success, False on fail. +*****************************************************************/ + +bool string_to_sid(struct dom_sid *sidout, const char *sidstr) +{ + const char *p; + char *q; + /* BIG NOTE: this function only does SIDS where the identauth is not >= 2^32 */ + uint32_t conv; + + if ((sidstr[0] != 'S' && sidstr[0] != 's') || sidstr[1] != '-') { + goto format_error; } - sidstr += 2; + ZERO_STRUCTP(sidout); - rev = strtol(sidstr, &p, 10); - if (*p != '-') { - return false; + /* Get the revision number. */ + p = sidstr + 2; + + if (!isdigit(*p)) { + goto format_error; } - sidstr = p+1; - ia = strtol(sidstr, &p, 10); - if (p == sidstr) { - return false; + conv = (uint32_t) strtoul(p, &q, 10); + if (!q || (*q != '-')) { + goto format_error; } - sidstr = p; + sidout->sid_rev_num = (uint8_t) conv; + q++; - num_sub_auths = 0; - for (i=0;sidstr[i];i++) { - if (sidstr[i] == '-') num_sub_auths++; + if (!isdigit(*q)) { + goto format_error; } - ret->sid_rev_num = rev; - ret->id_auth[0] = 0; - ret->id_auth[1] = 0; - ret->id_auth[2] = ia >> 24; - ret->id_auth[3] = ia >> 16; - ret->id_auth[4] = ia >> 8; - ret->id_auth[5] = ia; - ret->num_auths = num_sub_auths; - - for (i=0;iid_auth[0] = 0; + sidout->id_auth[1] = 0; + sidout->id_auth[2] = (conv & 0xff000000) >> 24; + sidout->id_auth[3] = (conv & 0x00ff0000) >> 16; + sidout->id_auth[4] = (conv & 0x0000ff00) >> 8; + sidout->id_auth[5] = (conv & 0x000000ff); + + sidout->num_auths = 0; + if (*q == '\0') { + return true; + } + + q++; + + while (true) { + char *end; + + if (!isdigit(*q)) { + goto format_error; } - sidstr++; - ret->sub_auths[i] = strtoul(sidstr, &p, 10); - if (p == sidstr) { + + conv = strtoul(q, &end, 10); + if (end == q) { + goto format_error; + } + + if (!sid_append_rid(sidout, conv)) { + DEBUG(3, ("Too many sid auths in %s\n", sidstr)); return false; } - sidstr = p; - } + q = end; + if (*q == '\0') { + break; + } + if (*q != '-') { + goto format_error; + } + q += 1; + } return true; + +format_error: + DEBUG(3, ("string_to_sid: SID %s is not in a valid format\n", sidstr)); + return false; +} + +bool dom_sid_parse(const char *sidstr, struct dom_sid *ret) +{ + return string_to_sid(ret, sidstr); } /* diff --git a/source3/lib/util_sid.c b/source3/lib/util_sid.c index 1f65f77991..b0b8d0ef72 100644 --- a/source3/lib/util_sid.c +++ b/source3/lib/util_sid.c @@ -194,112 +194,6 @@ char *sid_string_tos(const struct dom_sid *sid) return sid_string_talloc(talloc_tos(), sid); } -/***************************************************************** - Convert a string to a SID. Returns True on success, False on fail. -*****************************************************************/ - -bool string_to_sid(struct dom_sid *sidout, const char *sidstr) -{ - const char *p; - char *q; - /* BIG NOTE: this function only does SIDS where the identauth is not >= 2^32 */ - uint32_t conv; - - if ((sidstr[0] != 'S' && sidstr[0] != 's') || sidstr[1] != '-') { - goto format_error; - } - - ZERO_STRUCTP(sidout); - - /* Get the revision number. */ - p = sidstr + 2; - - if (!isdigit(*p)) { - goto format_error; - } - - conv = (uint32_t) strtoul(p, &q, 10); - if (!q || (*q != '-')) { - goto format_error; - } - sidout->sid_rev_num = (uint8_t) conv; - q++; - - if (!isdigit(*q)) { - goto format_error; - } - - /* get identauth */ - conv = (uint32_t) strtoul(q, &q, 10); - if (!q) { - goto format_error; - } else if (*q == '\0') { - /* Just id_auth, no subauths */ - } else if (*q != '-') { - goto format_error; - } - /* identauth in decimal should be < 2^32 */ - /* NOTE - the conv value is in big-endian format. */ - sidout->id_auth[0] = 0; - sidout->id_auth[1] = 0; - sidout->id_auth[2] = (conv & 0xff000000) >> 24; - sidout->id_auth[3] = (conv & 0x00ff0000) >> 16; - sidout->id_auth[4] = (conv & 0x0000ff00) >> 8; - sidout->id_auth[5] = (conv & 0x000000ff); - - sidout->num_auths = 0; - if (*q == '\0') { - return true; - } - - q++; - - while (true) { - char *end; - - if (!isdigit(*q)) { - goto format_error; - } - - conv = strtoul(q, &end, 10); - if (end == q) { - goto format_error; - } - - if (!sid_append_rid(sidout, conv)) { - DEBUG(3, ("Too many sid auths in %s\n", sidstr)); - return false; - } - - q = end; - if (*q == '\0') { - break; - } - if (*q != '-') { - goto format_error; - } - q += 1; - } - return true; - -format_error: - DEBUG(3, ("string_to_sid: SID %s is not in a valid format\n", sidstr)); - return false; -} - -/***************************************************************** - Add a rid to the end of a sid -*****************************************************************/ - -bool sid_append_rid(struct dom_sid *sid, uint32_t rid) -{ - if (sid->num_auths < ARRAY_SIZE(sid->sub_auths)) { - sid->sub_auths[sid->num_auths++] = rid; - return true; - } - return false; -} - bool sid_compose(struct dom_sid *dst, const struct dom_sid *domain_sid, uint32 rid) { sid_copy(dst, domain_sid);