The branch, v3-6-test has been updated
via c4a31cf Fix string_to_sid() to allow non '
via ea8f73f s3-torture Add tests to show that the dom_sid parsing was
faulty.
via b45b538 s3-util_sid Use the NDR parser to parse struct dom_sid
via dad0b14 libcli/security Use sid_append_rid() in dom_sid_append_rid()
via 4ac32a5 libcli/security Merge source3/ string_to_sid() to common
code
via 9e31c9a s3-util_sid use ARRAY_SIZE() to ensure we never overflow
the dom_sid
via 1ac4f6a s3-util_sid Accept S-1-5 as a SID (cherry picked from
commit 9d44688681bc196baf1bccbdf84092ffc0510bb7)
via 0dc0a81 s3-dom_sid Use C99 types in dom_sid handling
from f4c8bda Fix bug 7409 - Thousands of reduce_name: couldn't get
realpath.
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v3-6-test
- Log -----------------------------------------------------------------
commit c4a31cf4d6b1a7c342ed223bdbab3dbd21073f5d
Author: Jeremy Allison <j...@samba.org>
Date: Tue Sep 14 14:45:45 2010 -0700
Fix string_to_sid() to allow non '\0' termination of the string - allows
string_to_sid() to be used in formatted strings like
FOO/S-1-5-XXXX-YYYY/BAR.
Jeremy.
(cherry picked from commit 55b315094ef8a8ed691f9717c28cab301e17ef25)
commit ea8f73f2eac760ac723d1b7ef1ab66f40095e286
Author: Andrew Bartlett <abart...@samba.org>
Date: Sat Sep 4 14:13:31 2010 +1000
s3-torture Add tests to show that the dom_sid parsing was faulty.
Andrew Bartlett
(cherry picked from commit 15abd86d54c582edfec29dfd55c256b6565da569)
commit b45b538198480c8fe75ad2445a365da3abd80eb5
Author: Andrew Bartlett <abart...@samba.org>
Date: Sat Sep 4 14:11:46 2010 +1000
s3-util_sid Use the NDR parser to parse struct dom_sid
The manual parser failed to constrain the maximum number of
sub-authorities to 15, allowing an overflow of the array.
Andrew Bartlett
(cherry picked from commit 1892df6ca803aed94e91cbd7a12ca1b8470dfc89)
commit dad0b141403fdc576d4fb2e3735a861effa50bc2
Author: Andrew Bartlett <abart...@samba.org>
Date: Sat Sep 4 14:10:31 2010 +1000
libcli/security Use sid_append_rid() in dom_sid_append_rid()
This ensures that the maximum number of sub-authorities is respected,
otherwise we may run off the end of the array.
Andrew Bartlett
(cherry picked from commit 46f585e364fc1640cf01ba0c738c6c5559f0b4fd)
commit 4ac32a574bc00df3826d20efc4fcfd00c75ed866
Author: Andrew Bartlett <abart...@samba.org>
Date: Sat Sep 4 14:09:17 2010 +1000
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
(cherry picked from commit 51ecf796549287b7f10092778ffb52e018ae32fe)
commit 9e31c9a34a806bebd67f6bd722a7fdfeeede2e8f
Author: Andrew Bartlett <abart...@samba.org>
Date: Sat Sep 4 14:05:59 2010 +1000
s3-util_sid use ARRAY_SIZE() to ensure we never overflow the dom_sid
This ensures that this, unlike the MAXSUBAUTHS macro, can't get
out of sync with the structure.
Andrew Bartlett
(cherry picked from commit 72a8ea4d1545190bad85ee9f2216499e78b3625a)
commit 1ac4f6a4042962aae95c7ac00845035e0abfc646
Author: Andrew Bartlett <abart...@samba.org>
Date: Sat Sep 4 14:05:30 2010 +1000
s3-util_sid Accept S-1-5 as a SID
(cherry picked from commit 9d44688681bc196baf1bccbdf84092ffc0510bb7)
commit 0dc0a81c424a6a11ae8ae52086164dc37e4b3691
Author: Andrew Bartlett <abart...@samba.org>
Date: Sat Sep 4 14:04:55 2010 +1000
s3-dom_sid Use C99 types in dom_sid handling
Andrew Bartlett
(cherry picked from commit ce1e273a47105fcef71d054c0192b7985fd5b4f2)
-----------------------------------------------------------------------
Summary of changes:
libcli/security/dom_sid.c | 134 +++++++++++++++++++++++++++++++--------------
source3/lib/util_sid.c | 120 +++-------------------------------------
source3/torture/torture.c | 98 +++++++++++++++++++++++++++++++++
3 files changed, 198 insertions(+), 154 deletions(-)
Changeset truncated at 500 lines:
diff --git a/libcli/security/dom_sid.c b/libcli/security/dom_sid.c
index ef534e3..93f8871 100644
--- a/libcli/security/dom_sid.c
+++ b/libcli/security/dom_sid.c
@@ -85,60 +85,110 @@ 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;
+
+ ZERO_STRUCTP(sidout);
+
+ if ((sidstr[0] != 'S' && sidstr[0] != 's') || sidstr[1] != '-') {
+ goto format_error;
}
- sidstr += 2;
+ /* Get the revision number. */
+ p = sidstr + 2;
- rev = strtol(sidstr, &p, 10);
- if (*p != '-') {
- return false;
+ 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;i<num_sub_auths;i++) {
- if (sidstr[0] != '-') {
- return false;
+ /* get identauth */
+ conv = (uint32_t) strtoul(q, &q, 10);
+ 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 != '-') {
+ /* Just id_auth, no subauths */
+ return true;
+ }
+
+ q++;
+
+ while (true) {
+ char *end;
+
+ if (!isdigit(*q)) {
+ goto format_error;
+ }
+
+ conv = strtoul(q, &end, 10);
+ if (end == q) {
+ goto format_error;
}
- sidstr++;
- ret->sub_auths[i] = strtoul(sidstr, &p, 10);
- if (p == sidstr) {
+
+ if (!sid_append_rid(sidout, conv)) {
+ DEBUG(3, ("Too many sid auths in %s\n", sidstr));
return false;
}
- sidstr = p;
- }
+ q = end;
+ if (*q != '-') {
+ break;
+ }
+ 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);
}
/*
@@ -217,13 +267,13 @@ struct dom_sid *dom_sid_add_rid(TALLOC_CTX *mem_ctx,
{
struct dom_sid *sid;
- sid = talloc(mem_ctx, struct dom_sid);
+ sid = dom_sid_dup(mem_ctx, domain_sid);
if (!sid) return NULL;
- *sid = *domain_sid;
-
- sid->sub_auths[sid->num_auths] = rid;
- sid->num_auths++;
+ if (!sid_append_rid(sid, rid)) {
+ talloc_free(sid);
+ return NULL;
+ }
return sid;
}
diff --git a/source3/lib/util_sid.c b/source3/lib/util_sid.c
index 31a4c06..3a32146 100644
--- a/source3/lib/util_sid.c
+++ b/source3/lib/util_sid.c
@@ -200,104 +200,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 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) strtoul(p, &q, 10);
- if (!q || (*q != '-')) {
- goto format_error;
- }
- sidout->sid_rev_num = (uint8) conv;
- q++;
-
- if (!isdigit(*q)) {
- goto format_error;
- }
-
- /* get identauth */
- conv = (uint32) strtoul(q, &q, 10);
- if (!q || (*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);
-
- q++;
- sidout->num_auths = 0;
-
- 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 rid)
-{
- if (sid->num_auths < MAXSUBAUTHS) {
- 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);
@@ -401,20 +303,14 @@ bool sid_linearize(char *outbuf, size_t len, const struct
dom_sid *sid)
bool sid_parse(const char *inbuf, size_t len, struct dom_sid *sid)
{
- int i;
- if (len < 8)
- return False;
-
- ZERO_STRUCTP(sid);
-
- sid->sid_rev_num = CVAL(inbuf, 0);
- sid->num_auths = CVAL(inbuf, 1);
- memcpy(sid->id_auth, inbuf+2, 6);
- if (len < 8 + sid->num_auths*4)
- return False;
- for (i=0;i<sid->num_auths;i++)
- sid->sub_auths[i] = IVAL(inbuf, 8+i*4);
- return True;
+ enum ndr_err_code ndr_err;
+ DATA_BLOB in = data_blob_const(inbuf, len);
+ ndr_err = ndr_pull_struct_blob_all(&in, NULL, sid,
+
(ndr_pull_flags_fn_t)ndr_pull_dom_sid);
+ if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+ return false;
+ }
+ return true;
}
/*****************************************************************
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 384a57a..3e1e198 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -6836,6 +6836,103 @@ static bool run_local_string_to_sid(int dummy) {
return true;
}
+static bool run_local_binary_to_sid(int dummy) {
+ struct dom_sid *sid = talloc(NULL, struct dom_sid);
+ static const char good_binary_sid[] = {
+ 0x1, /* revision number */
+ 15, /* num auths */
+ 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, /* id_auth */
+ 0x1, 0x1, 0x1, 0x1, /* auth[0] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[1] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[2] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[3] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[4] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[5] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[6] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[7] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[8] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[9] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[10] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[11] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[12] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[13] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[14] */
+ };
+
+ static const char long_binary_sid[] = {
+ 0x1, /* revision number */
+ 15, /* num auths */
+ 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, /* id_auth */
+ 0x1, 0x1, 0x1, 0x1, /* auth[0] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[1] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[2] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[3] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[4] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[5] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[6] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[7] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[8] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[9] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[10] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[11] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[12] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[13] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[14] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[15] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[16] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[17] */
+ };
+
+ static const char long_binary_sid2[] = {
+ 0x1, /* revision number */
+ 32, /* num auths */
+ 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, /* id_auth */
+ 0x1, 0x1, 0x1, 0x1, /* auth[0] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[1] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[2] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[3] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[4] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[5] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[6] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[7] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[8] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[9] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[10] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[11] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[12] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[13] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[14] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[15] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[16] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[17] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[18] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[19] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[20] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[21] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[22] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[23] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[24] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[25] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[26] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[27] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[28] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[29] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[30] */
+ 0x1, 0x1, 0x1, 0x1, /* auth[31] */
+ };
+
+ if (!sid_parse(good_binary_sid, sizeof(good_binary_sid), sid)) {
+ return false;
+ }
+ if (sid_parse(long_binary_sid2, sizeof(long_binary_sid2), sid)) {
+ return false;
+ }
+ if (sid_parse(long_binary_sid, sizeof(long_binary_sid), sid)) {
+ return false;
+ }
+ return true;
+}
+
/* Split a path name into filename and stream name components. Canonicalise
* such that an implicit $DATA token is always explicit.
*
@@ -7568,6 +7665,7 @@ static struct {
{ "LOCAL-STREAM-NAME", run_local_stream_name, 0},
{ "LOCAL-WBCLIENT", run_local_wbclient, 0},
{ "LOCAL-string_to_sid", run_local_string_to_sid, 0},
+ { "LOCAL-binary_to_sid", run_local_binary_to_sid, 0},
{ "LOCAL-DBTRANS", run_local_dbtrans, 0},
{ "LOCAL-TEVENT-SELECT", run_local_tevent_select, 0},
{NULL, NULL, 0}};
--
Samba Shared Repository
