[SCM] Samba Shared Repository - branch v3-6-test updated

Jeremy Allison jra at samba.org
Tue Sep 14 15:51:45 MDT 2010


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 <jra at 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 <abartlet at 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 <abartlet at 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 <abartlet at 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 <abartlet at 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 <abartlet at 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 <abartlet at 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 <abartlet at 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


More information about the samba-cvs mailing list