[SCM] Samba Shared Repository - branch v4-6-stable updated

Karolin Seeger kseeger at samba.org
Tue Aug 14 07:49:15 UTC 2018


The branch, v4-6-stable has been updated
       via  18df99b VERSION: Disable GIT_SNAPSHOT for the 4.6.16 release.
       via  cd2839a WHATSNEW: Add release notes for Samba 4.6.16.
       via  9f166c0 CVE-2018-10919 tests: Add extra test for dirsync deleted object corner-case
       via  246e79f CVE-2018-10919 acl_read: Fix unauthorized attribute access via searches
       via  9605ecc CVE-2018-10919 acl_read: Flip the logic in the dirsync check
       via  533106a CVE-2018-10919 acl_read: Small refactor to aclread_callback()
       via  fa7bcea CVE-2018-10919 acl_read: Split access_mask logic out into helper function
       via  f6cbad5 CVE-2018-10919 security: Fix checking of object-specific CONTROL_ACCESS rights
       via  873ccd0 CVE-2018-10919 tests: test ldap searches for non-existent attributes.
       via  924f87c CVE-2018-10919 tests: Add test case for object visibility with limited rights
       via  3388706 CVE-2018-10919 tests: Add tests for guessing confidential attributes
       via  010d1f1 CVE-2018-10919 security: Add more comments to the object-specific access checks
       via  2878c22 CVE-2018-10919 security: Move object-specific access checks into separate function
       via  2711b66 CVE-2018-10858: libsmb: Harden smbc_readdir_internal() against returns from malicious servers.
       via  6936d3e CVE-2018-10858: libsmb: Ensure smbc_urlencode() can't overwrite passed in buffer.
       via  30428f3 VERSION: Bump version up to 4.6.16...
      from  c4d44b9 VERSION: Disable GIT_SNAPSHOT for the 4.6.15 release.

https://git.samba.org/?p=samba.git;a=shortlog;h=v4-6-stable


- Log -----------------------------------------------------------------
commit 18df99ba0bfc466b877d5875bef3ab1279b0e7dc
Author: Karolin Seeger <kseeger at samba.org>
Date:   Mon Aug 13 09:25:13 2018 +0200

    VERSION: Disable GIT_SNAPSHOT for the 4.6.16 release.
    
    o  CVE-2018-10858 (Insufficient input validation on client directory
                       listing in libsmbclient.)
    o  CVE-2018-10919 (Confidential attribute disclosure from the AD LDAP
                       server.)
    
    Signed-off-by: Karolin Seeger <kseeger at samba.org>

commit cd2839a1012c29bd72ad5f85884c93d5ac37442e
Author: Karolin Seeger <kseeger at samba.org>
Date:   Mon Aug 13 09:24:08 2018 +0200

    WHATSNEW: Add release notes for Samba 4.6.16.
    
    o  CVE-2018-10858 (Insufficient input validation on client directory
                       listing in libsmbclient.)
    o  CVE-2018-10919 (Confidential attribute disclosure from the AD LDAP
                       server.)
    
    Signed-off-by: Karolin Seeger <kseeger at samba.org>

commit 9f166c0222315393fef9b456f246dfae5a12439c
Author: Tim Beale <timbeale at catalyst.net.nz>
Date:   Wed Aug 1 13:51:42 2018 +1200

    CVE-2018-10919 tests: Add extra test for dirsync deleted object corner-case
    
    The acl_read.c code contains a special case to allow dirsync to
    work-around having insufficient access rights. We had a concern that
    the dirsync module could leak sensitive information for deleted objects.
    This patch adds a test-case to prove whether or not this is happening.
    
    The new test case is similar to the existing dirsync test except:
    - We make the confidential attribute also preserve-on-delete, so it
      hangs around for deleted objcts. Because the attributes now persist
      across test case runs, I've used a different attribute to normal.
      (Technically, the dirsync search expressions are now specific enough
      that the regular attribute could be used, but it would make things
      quite fragile if someone tried to add a new test case).
    - To handle searching for deleted objects, the search expressions are
      now more complicated. Currently dirsync adds an extra-filter to the
      '!' searches to exclude deleted objects, i.e. samaccountname matches
      the test-objects AND the object is not deleted. We now extend this to
      include deleted objects with lastKnownParent equal to the test OU.
      The search expression matches either case so that we can use the same
      expression throughout the test (regardless of whether the object is
      deleted yet or not).
    
    This test proves that the dirsync corner-case does not actually leak
    sensitive information on Samba. This is due to a bug in the dirsync
    code - when the buggy line is removed, this new test promptly fails.
    Test also passes against Windows.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13434
    
    Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 246e79f6ed34e6585eb22bb68aa558b85e0a6522
Author: Tim Beale <timbeale at catalyst.net.nz>
Date:   Fri Jul 20 15:42:36 2018 +1200

    CVE-2018-10919 acl_read: Fix unauthorized attribute access via searches
    
    A user that doesn't have access to view an attribute can still guess the
    attribute's value via repeated LDAP searches. This affects confidential
    attributes, as well as ACLs applied to an object/attribute to deny
    access.
    
    Currently the code will hide objects if the attribute filter contains an
    attribute they are not authorized to see. However, the code still
    returns objects as results if confidential attribute is in the search
    expression itself, but not in the attribute filter.
    
    To fix this problem we have to check the access rights on the attributes
    in the search-tree, as well as the attributes returned in the message.
    
    Points of note:
    - I've preserved the existing dirsync logic (the dirsync module code
      suppresses the result as long as the replPropertyMetaData attribute is
      removed). However, there doesn't appear to be any test that highlights
      that this functionality is required for dirsync.
    - To avoid this fix breaking the acl.py tests, we need to still permit
      searches like 'objectClass=*', even though we don't have Read Property
      access rights for the objectClass attribute. The logic that Windows
      uses does not appear to be clearly documented, so I've made a best
      guess that seems to mirror Windows behaviour.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13434
    
    Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 9605ecc7e1a79fa1b7cdf9f53048595ee00008b2
Author: Tim Beale <timbeale at catalyst.net.nz>
Date:   Mon Jul 30 16:00:15 2018 +1200

    CVE-2018-10919 acl_read: Flip the logic in the dirsync check
    
    This better reflects the special case we're making for dirsync, and gets
    rid of a 'if-else' clause.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13434
    
    Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 533106ae9c210a3d7c501f239e7de9d0966f27b9
Author: Tim Beale <timbeale at catalyst.net.nz>
Date:   Thu Jul 26 12:20:49 2018 +1200

    CVE-2018-10919 acl_read: Small refactor to aclread_callback()
    
    Flip the dirsync check (to avoid a double negative), and use a helper
    boolean variable.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13434
    
    Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit fa7bcea388b310f461f1f2c1788687523c6f1a18
Author: Tim Beale <timbeale at catalyst.net.nz>
Date:   Tue Jul 31 16:00:12 2018 +1200

    CVE-2018-10919 acl_read: Split access_mask logic out into helper function
    
    So we can re-use the same logic laster for checking the search-ops.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13434
    
    Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit f6cbad5f5039570973ecb7dfa4686eb384fe6f7f
Author: Tim Beale <timbeale at catalyst.net.nz>
Date:   Fri Jul 20 13:01:00 2018 +1200

    CVE-2018-10919 security: Fix checking of object-specific CONTROL_ACCESS rights
    
    An 'Object Access Allowed' ACE that assigned 'Control Access' (CR)
    rights to a specific attribute would not actually grant access.
    
    What was happening was the remaining_access mask for the object_tree
    nodes would be Read Property (RP) + Control Access (CR). The ACE mapped
    to the schemaIDGUID for a given attribute, which would end up being a
    child node in the tree. So the CR bit was cleared for a child node, but
    not the rest of the tree. We would then check the user had the RP access
    right, which it did. However, the RP right was cleared for another node
    in the tree, which still had the CR bit set in its remaining_access
    bitmap, so Samba would not grant access.
    
    Generally, the remaining_access only ever has one bit set, which means
    this isn't a problem normally. However, in the Control Access case there
    are 2 separate bits being checked, i.e. RP + CR.
    
    One option to fix this problem would be to clear the remaining_access
    for the tree instead of just the node. However, the Windows spec is
    actually pretty clear on this: if the ACE has a CR right present, then
    you can stop any further access checks.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13434
    
    Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 873ccd079f2d21bba22624a79c6bf14bc38e80ad
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Fri Aug 3 15:51:28 2018 +1200

    CVE-2018-10919 tests: test ldap searches for non-existent attributes.
    
    It is perfectly legal to search LDAP for an attribute that is not part
    of the schema.  That part of the query should simply not match.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13434
    
    Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 924f87cb74a383f2dc1acfc33c1021d8399b5e40
Author: Tim Beale <timbeale at catalyst.net.nz>
Date:   Tue Jul 31 14:14:20 2018 +1200

    CVE-2018-10919 tests: Add test case for object visibility with limited rights
    
    Currently Samba is a bit disclosive with LDB_OP_PRESENT (i.e.
    attribute=*) searches compared to Windows.
    
    All the acl.py tests are based on objectClass=* searches, where Windows
    will happily tell a user about objects they have List Contents rights,
    but not Read Property rights for. However, if you change the attribute
    being searched for, suddenly the objects are no longer visible on
    Windows (whereas they are on Samba).
    
    This is a problem, because Samba can tell you about which objects have
    confidential attributes, which in itself could be disclosive.
    
    This patch adds a acl.py test-case that highlights this behaviour. The
    test passes against Windows but fails against Samba.
    
    Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 33887063b0623a37e954ce318cbc5cd21e079a72
Author: Tim Beale <timbeale at catalyst.net.nz>
Date:   Mon Jul 9 15:57:59 2018 +1200

    CVE-2018-10919 tests: Add tests for guessing confidential attributes
    
    Adds tests that assert that a confidential attribute cannot be guessed
    by an unprivileged user through wildcard DB searches.
    
    The tests basically consist of a set of DB searches/assertions that
    get run for:
    - basic searches against a confidential attribute
    - confidential attributes that get overridden by giving access to the
      user via an ACE (run against a variety of ACEs)
    - protecting a non-confidential attribute via an ACL that denies read-
      access (run against a variety of ACEs)
    - querying confidential attributes via the dirsync controls
    
    These tests all pass when run against a Windows Dc and all fail against
    a Samba DC.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13434
    
    Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 010d1f1810b90f1eb7b86340aacd727610d8e585
Author: Tim Beale <timbeale at catalyst.net.nz>
Date:   Fri Jul 20 13:13:50 2018 +1200

    CVE-2018-10919 security: Add more comments to the object-specific access checks
    
    Reading the spec and then reading the code makes sense, but we could
    comment the code more so it makes sense on its own.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13434
    
    Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 2878c22070df797e633b8966bb9b129a18865d69
Author: Tim Beale <timbeale at catalyst.net.nz>
Date:   Thu Jul 19 16:03:36 2018 +1200

    CVE-2018-10919 security: Move object-specific access checks into separate function
    
    Object-specific access checks refer to a specific section of the
    MS-ADTS, and the code closely matches the spec. We need to extend this
    logic to properly handle the Control-Access Right (CR), so it makes
    sense to split the logic out into its own function.
    
    This patch just moves the code, and should not alter the logic (apart
    from ading in the boolean grant_access return variable.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13434
    
    Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 2711b6600ee3b8b51b0cbf5736a7c588390d5314
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Jun 15 15:08:17 2018 -0700

    CVE-2018-10858: libsmb: Harden smbc_readdir_internal() against returns from malicious servers.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13453
    
    CVE-2018-10858: Insufficient input validation on client directory
    		listing in libsmbclient.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit 6936d3e2f2d8eb183f89dd3402403de1a45a5d08
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Jun 15 15:07:17 2018 -0700

    CVE-2018-10858: libsmb: Ensure smbc_urlencode() can't overwrite passed in buffer.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13453
    
    CVE-2018-10858: Insufficient input validation on client directory
    		listing in libsmbclient.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit 30428f36f79b0cc4133a7f640b334c566e1ef525
Author: Karolin Seeger <kseeger at samba.org>
Date:   Tue Apr 10 21:22:32 2018 +0200

    VERSION: Bump version up to 4.6.16...
    
    and re-enable GIT_SNAPSHOT.
    
    Signed-off-by: Karolin Seeger <kseeger at samba.org>
    (cherry picked from commit 7705a4d471a427041616a9897158474d8a5ff457)

-----------------------------------------------------------------------

Summary of changes:
 VERSION                                        |    2 +-
 WHATSNEW.txt                                   |   66 +-
 libcli/security/access_check.c                 |  110 ++-
 source3/libsmb/libsmb_dir.c                    |   57 +-
 source3/libsmb/libsmb_path.c                   |    9 +-
 source4/dsdb/samdb/ldb_modules/acl_read.c      |  333 +++++++-
 source4/dsdb/tests/python/acl.py               |   68 ++
 source4/dsdb/tests/python/confidential_attr.py | 1025 ++++++++++++++++++++++++
 source4/dsdb/tests/python/ldap.py              |    9 +
 source4/selftest/tests.py                      |    3 +
 10 files changed, 1608 insertions(+), 74 deletions(-)
 create mode 100755 source4/dsdb/tests/python/confidential_attr.py


Changeset truncated at 500 lines:

diff --git a/VERSION b/VERSION
index e657759..da35544 100644
--- a/VERSION
+++ b/VERSION
@@ -25,7 +25,7 @@
 ########################################################
 SAMBA_VERSION_MAJOR=4
 SAMBA_VERSION_MINOR=6
-SAMBA_VERSION_RELEASE=15
+SAMBA_VERSION_RELEASE=16
 
 ########################################################
 # If a official release has a serious bug              #
diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index fa673c3..d0c0533 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -1,4 +1,66 @@
                    ==============================
+                   Release Notes for Samba 4.6.16
+                           August 14, 2018
+                   ==============================
+
+
+This is a security release in order to address the following defects:
+
+o  CVE-2018-10858 (Insufficient input validation on client directory
+		   listing in libsmbclient.)
+o  CVE-2018-10919 (Confidential attribute disclosure from the AD LDAP
+		   server.)
+
+
+=======
+Details
+=======
+
+o  CVE-2018-10858:
+   A malicious server could return a directory entry that could corrupt
+   libsmbclient memory.
+
+o  CVE-2018-10919:
+   Missing access control checks allow discovery of confidential attribute
+   values via authenticated LDAP search expressions.
+
+
+Changes since 4.6.15:
+--------------------
+
+o  Jeremy Allison <jra at samba.org>
+   * BUG 13453: CVE-2018-10858: libsmb: Harden smbc_readdir_internal() against
+     returns from malicious servers.
+
+o  Tim Beale <timbeale at catalyst.net.nz>
+   * BUG 13434: CVE-2018-10919: acl_read: Fix unauthorized attribute access via
+     searches.
+
+
+#######################################
+Reporting bugs & Development Discussion
+#######################################
+
+Please discuss this release on the samba-technical mailing list or by
+joining the #samba-technical IRC channel on irc.freenode.net.
+
+If you do report problems then please try to send high quality
+feedback. If you don't provide vital information to help us track down
+the problem then you will probably be ignored.  All bug reports should
+be filed under the "Samba 4.1 and newer" product in the project's Bugzilla
+database (https://bugzilla.samba.org/).
+
+
+======================================================================
+== Our Code, Our Bugs, Our Responsibility.
+== The Samba Team
+======================================================================
+
+
+Release notes for older releases follow:
+----------------------------------------
+
+                   ==============================
                    Release Notes for Samba 4.6.15
                            April 13, 2018
                    ==============================
@@ -70,8 +132,8 @@ database (https://bugzilla.samba.org/).
 ======================================================================
 
 
-Release notes for older releases follow:
-----------------------------------------
+----------------------------------------------------------------------
+
 
                    ==============================
                    Release Notes for Samba 4.6.14
diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c
index b4c850b..03a7dca 100644
--- a/libcli/security/access_check.c
+++ b/libcli/security/access_check.c
@@ -375,6 +375,81 @@ static const struct GUID *get_ace_object_type(struct security_ace *ace)
 }
 
 /**
+ * Evaluates access rights specified in a object-specific ACE for an AD object.
+ * This logic corresponds to MS-ADTS 5.1.3.3.3 Checking Object-Specific Access.
+ * @param[in] ace - the ACE being processed
+ * @param[in/out] tree - remaining_access gets updated for the tree
+ * @param[out] grant_access - set to true if the ACE grants sufficient access
+ *                            rights to the object/attribute
+ * @returns NT_STATUS_OK, unless access was denied
+ */
+static NTSTATUS check_object_specific_access(struct security_ace *ace,
+					     struct object_tree *tree,
+					     bool *grant_access)
+{
+	struct object_tree *node = NULL;
+	const struct GUID *type = NULL;
+
+	*grant_access = false;
+
+	/* if no tree was supplied, we can't do object-specific access checks */
+	if (!tree) {
+		return NT_STATUS_OK;
+	}
+
+	/* Get the ObjectType GUID this ACE applies to */
+	type = get_ace_object_type(ace);
+
+	/*
+	 * If the ACE doesn't have a type, then apply it to the whole tree, i.e.
+	 * treat 'OA' ACEs as 'A' and 'OD' as 'D'
+	 */
+	if (!type) {
+		node = tree;
+	} else {
+
+		/* skip it if the ACE's ObjectType GUID is not in the tree */
+		node = get_object_tree_by_GUID(tree, type);
+		if (!node) {
+			return NT_STATUS_OK;
+		}
+	}
+
+	if (ace->type == SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT) {
+
+		/* apply the access rights to this node, and any children */
+		object_tree_modify_access(node, ace->access_mask);
+
+		/*
+		 * Currently all nodes in the tree request the same access mask,
+		 * so we can use any node to check if processing this ACE now
+		 * means the requested access has been granted
+		 */
+		if (node->remaining_access == 0) {
+			*grant_access = true;
+			return NT_STATUS_OK;
+		}
+
+		/*
+		 * As per 5.1.3.3.4 Checking Control Access Right-Based Access,
+		 * if the CONTROL_ACCESS right is present, then we can grant
+		 * access and stop any further access checks
+		 */
+		if (ace->access_mask & SEC_ADS_CONTROL_ACCESS) {
+			*grant_access = true;
+			return NT_STATUS_OK;
+		}
+	} else {
+
+		/* this ACE denies access to the requested object/attribute */
+		if (node->remaining_access & ace->access_mask){
+			return NT_STATUS_ACCESS_DENIED;
+		}
+	}
+	return NT_STATUS_OK;
+}
+
+/**
  * @brief Perform directoryservice (DS) related access checks for a given user
  *
  * Perform DS access checks for the user represented by its security_token, on
@@ -405,8 +480,6 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd,
 {
 	uint32_t i;
 	uint32_t bits_remaining;
-	struct object_tree *node;
-	const struct GUID *type;
 	struct dom_sid self_sid;
 
 	dom_sid_parse(SID_NT_SELF, &self_sid);
@@ -456,6 +529,8 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd,
 	for (i=0; bits_remaining && i < sd->dacl->num_aces; i++) {
 		struct dom_sid *trustee;
 		struct security_ace *ace = &sd->dacl->aces[i];
+		NTSTATUS status;
+		bool grant_access = false;
 
 		if (ace->flags & SEC_ACE_FLAG_INHERIT_ONLY) {
 			continue;
@@ -486,34 +561,15 @@ NTSTATUS sec_access_check_ds(const struct security_descriptor *sd,
 			break;
 		case SEC_ACE_TYPE_ACCESS_DENIED_OBJECT:
 		case SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT:
-			/*
-			 * check only in case we have provided a tree,
-			 * the ACE has an object type and that type
-			 * is in the tree
-			 */
-			type = get_ace_object_type(ace);
-
-			if (!tree) {
-				continue;
-			}
+			status = check_object_specific_access(ace, tree,
+							      &grant_access);
 
-			if (!type) {
-				node = tree;
-			} else {
-				if (!(node = get_object_tree_by_GUID(tree, type))) {
-					continue;
-				}
+			if (!NT_STATUS_IS_OK(status)) {
+				return status;
 			}
 
-			if (ace->type == SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT) {
-				object_tree_modify_access(node, ace->access_mask);
-				if (node->remaining_access == 0) {
-					return NT_STATUS_OK;
-				}
-			} else {
-				if (node->remaining_access & ace->access_mask){
-					return NT_STATUS_ACCESS_DENIED;
-				}
+			if (grant_access) {
+				return NT_STATUS_OK;
 			}
 			break;
 		default:	/* Other ACE types not handled/supported */
diff --git a/source3/libsmb/libsmb_dir.c b/source3/libsmb/libsmb_dir.c
index 6314591..96eb163 100644
--- a/source3/libsmb/libsmb_dir.c
+++ b/source3/libsmb/libsmb_dir.c
@@ -930,27 +930,47 @@ SMBC_closedir_ctx(SMBCCTX *context,
 
 }
 
-static void
+static int
 smbc_readdir_internal(SMBCCTX * context,
                       struct smbc_dirent *dest,
                       struct smbc_dirent *src,
                       int max_namebuf_len)
 {
         if (smbc_getOptionUrlEncodeReaddirEntries(context)) {
+		int remaining_len;
 
                 /* url-encode the name.  get back remaining buffer space */
-                max_namebuf_len =
+                remaining_len =
                         smbc_urlencode(dest->name, src->name, max_namebuf_len);
 
+		/* -1 means no null termination. */
+		if (remaining_len < 0) {
+			return -1;
+		}
+
                 /* We now know the name length */
                 dest->namelen = strlen(dest->name);
 
+		if (dest->namelen + 1 < 1) {
+			/* Integer wrap. */
+			return -1;
+		}
+
+		if (dest->namelen + 1 >= max_namebuf_len) {
+			/* Out of space for comment. */
+			return -1;
+		}
+
                 /* Save the pointer to the beginning of the comment */
                 dest->comment = dest->name + dest->namelen + 1;
 
+		if (remaining_len < 1) {
+			/* No room for comment null termination. */
+			return -1;
+		}
+
                 /* Copy the comment */
-                strncpy(dest->comment, src->comment, max_namebuf_len - 1);
-                dest->comment[max_namebuf_len - 1] = '\0';
+                strlcpy(dest->comment, src->comment, remaining_len);
 
                 /* Save other fields */
                 dest->smbc_type = src->smbc_type;
@@ -960,10 +980,21 @@ smbc_readdir_internal(SMBCCTX * context,
         } else {
 
                 /* No encoding.  Just copy the entry as is. */
+		if (src->dirlen > max_namebuf_len) {
+			return -1;
+		}
                 memcpy(dest, src, src->dirlen);
+		if (src->namelen + 1 < 1) {
+			/* Integer wrap */
+			return -1;
+		}
+		if (src->namelen + 1 >= max_namebuf_len) {
+			/* Comment off the end. */
+			return -1;
+		}
                 dest->comment = (char *)(&dest->name + src->namelen + 1);
         }
-
+	return 0;
 }
 
 /*
@@ -975,6 +1006,7 @@ SMBC_readdir_ctx(SMBCCTX *context,
                  SMBCFILE *dir)
 {
         int maxlen;
+	int ret;
 	struct smbc_dirent *dirp, *dirent;
 	TALLOC_CTX *frame = talloc_stackframe();
 
@@ -1024,7 +1056,12 @@ SMBC_readdir_ctx(SMBCCTX *context,
         dirp = &context->internal->dirent;
         maxlen = sizeof(context->internal->_dirent_name);
 
-        smbc_readdir_internal(context, dirp, dirent, maxlen);
+        ret = smbc_readdir_internal(context, dirp, dirent, maxlen);
+	if (ret == -1) {
+		errno = EINVAL;
+		TALLOC_FREE(frame);
+                return NULL;
+	}
 
         dir->dir_next = dir->dir_next->next;
 
@@ -1082,6 +1119,7 @@ SMBC_getdents_ctx(SMBCCTX *context,
 	 */
 
 	while ((dirlist = dir->dir_next)) {
+		int ret;
 		struct smbc_dirent *dirent;
 		struct smbc_dirent *currentEntry = (struct smbc_dirent *)ndir;
 
@@ -1096,8 +1134,13 @@ SMBC_getdents_ctx(SMBCCTX *context,
                 /* Do urlencoding of next entry, if so selected */
                 dirent = &context->internal->dirent;
                 maxlen = sizeof(context->internal->_dirent_name);
-                smbc_readdir_internal(context, dirent,
+		ret = smbc_readdir_internal(context, dirent,
                                       dirlist->dirent, maxlen);
+		if (ret == -1) {
+			errno = EINVAL;
+			TALLOC_FREE(frame);
+			return -1;
+		}
 
                 reqd = dirent->dirlen;
 
diff --git a/source3/libsmb/libsmb_path.c b/source3/libsmb/libsmb_path.c
index 01b0a61..5b53b38 100644
--- a/source3/libsmb/libsmb_path.c
+++ b/source3/libsmb/libsmb_path.c
@@ -173,8 +173,13 @@ smbc_urlencode(char *dest,
                 }
         }
 
-        *dest++ = '\0';
-        max_dest_len--;
+	if (max_dest_len <= 0) {
+		/* Ensure we return -1 if no null termination. */
+		return -1;
+	}
+
+	*dest++ = '\0';
+	max_dest_len--;
 
         return max_dest_len;
 }
diff --git a/source4/dsdb/samdb/ldb_modules/acl_read.c b/source4/dsdb/samdb/ldb_modules/acl_read.c
index f15633f..eb8676d 100644
--- a/source4/dsdb/samdb/ldb_modules/acl_read.c
+++ b/source4/dsdb/samdb/ldb_modules/acl_read.c
@@ -38,6 +38,8 @@
 #include "param/param.h"
 #include "dsdb/samdb/ldb_modules/util.h"
 
+/* The attributeSecurityGuid for the Public Information Property-Set */
+#define PUBLIC_INFO_PROPERTY_SET "e48d0154-bcf8-11d1-8702-00c04fb96050"
 
 struct aclread_context {
 	struct ldb_module *module;
@@ -64,6 +66,254 @@ static bool aclread_is_inaccessible(struct ldb_message_element *el) {
 	return el->flags & LDB_FLAG_INTERNAL_INACCESSIBLE_ATTRIBUTE;
 }
 
+/*
+ * Returns the access mask required to read a given attribute
+ */
+static uint32_t get_attr_access_mask(const struct dsdb_attribute *attr,
+				     uint32_t sd_flags)
+{
+
+	uint32_t access_mask = 0;
+	bool is_sd;
+
+	/* nTSecurityDescriptor is a special case */
+	is_sd = (ldb_attr_cmp("nTSecurityDescriptor",
+			      attr->lDAPDisplayName) == 0);
+
+	if (is_sd) {
+		if (sd_flags & (SECINFO_OWNER|SECINFO_GROUP)) {
+			access_mask |= SEC_STD_READ_CONTROL;
+		}
+		if (sd_flags & SECINFO_DACL) {
+			access_mask |= SEC_STD_READ_CONTROL;
+		}
+		if (sd_flags & SECINFO_SACL) {
+			access_mask |= SEC_FLAG_SYSTEM_SECURITY;
+		}
+	} else {
+		access_mask = SEC_ADS_READ_PROP;
+	}
+
+	if (attr->searchFlags & SEARCH_FLAG_CONFIDENTIAL) {
+		access_mask |= SEC_ADS_CONTROL_ACCESS;
+	}
+
+	return access_mask;
+}
+
+/* helper struct for traversing the attributes in the search-tree */
+struct parse_tree_aclread_ctx {
+	struct aclread_context *ac;
+	TALLOC_CTX *mem_ctx;
+	struct dom_sid *sid;
+	struct ldb_dn *dn;
+	struct security_descriptor *sd;
+	const struct dsdb_class *objectclass;
+	bool suppress_result;
+};
+
+/*
+ * Checks that the user has sufficient access rights to view an attribute
+ */
+static int check_attr_access_rights(TALLOC_CTX *mem_ctx, const char *attr_name,
+				    struct aclread_context *ac,
+				    struct security_descriptor *sd,
+				    const struct dsdb_class *objectclass,
+				    struct dom_sid *sid, struct ldb_dn *dn,
+				    bool *is_public_info)
+{
+	int ret;
+	const struct dsdb_attribute *attr = NULL;
+	uint32_t access_mask;
+	struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
+
+	*is_public_info = false;
+
+	attr = dsdb_attribute_by_lDAPDisplayName(ac->schema, attr_name);
+	if (!attr) {
+		ldb_debug_set(ldb,
+			      LDB_DEBUG_TRACE,
+			      "acl_read: %s cannot find attr[%s] in schema,"
+			      "ignoring\n",
+			      ldb_dn_get_linearized(dn), attr_name);
+		return LDB_SUCCESS;
+	}
+
+	/*
+	 * If we have no Read Property (RP) rights for a child object, it should
+	 * still appear as a visible object in 'objectClass=*' searches,
+	 * as long as we have List Contents (LC) rights for it.
+	 * This is needed for the acl.py tests (e.g. test_search1()).
+	 * I couldn't find the Windows behaviour documented in the specs, so
+	 * this is a guess, but it seems to only apply to attributes in the
+	 * Public Information Property Set that have the systemOnly flag set to
+	 * TRUE. (This makes sense in a way, as it's not disclosive to find out
+	 * that a child object has a 'objectClass' or 'name' attribute, as every
+	 * object has these attributes).
+	 */
+	if (attr->systemOnly) {
+		struct GUID public_info_guid;
+		NTSTATUS status;
+
+		status = GUID_from_string(PUBLIC_INFO_PROPERTY_SET,
+					  &public_info_guid);
+		if (!NT_STATUS_IS_OK(status)) {
+			ldb_set_errstring(ldb, "Public Info GUID parse error");
+			return LDB_ERR_OPERATIONS_ERROR;
+		}
+
+		if (GUID_compare(&attr->attributeSecurityGUID,
+				 &public_info_guid) == 0) {
+			*is_public_info = true;


-- 
Samba Shared Repository



More information about the samba-cvs mailing list