[SCM] Samba Shared Repository - branch master updated

[Jeremy Allison]

The branch, master has been updated
       via  277eac1 lsa4_srv: Factor out dcesrc_lsa_valid_AccountRight()
      from  aa43d0d source3 smdb: fix null pointer dereference

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 277eac1a8e7443bae125e030000309e7eeea2799
Author: Volker Lendecke <vl at samba.org>
Date:   Thu Apr 27 13:37:42 2017 +0200

    lsa4_srv: Factor out dcesrc_lsa_valid_AccountRight()
    
    The previous code in dcesrv_lsa_AddRemoveAccountRights had the following snippet:
    
    if (sec_privilege_id(rights->names[i].string) == SEC_PRIV_INVALID) {
            if (sec_right_bit(rights->names[i].string) == 0) {
                    talloc_free(msg);
                    return NT_STATUS_NO_SUCH_PRIVILEGE;
            }
            talloc_free(msg);
            return NT_STATUS_NO_SUCH_PRIVILEGE;
    }
    
    If I'm not mistaken, the inner if-statement is essentially dead code,
    as regardless of the outcome of the if-condition we execute the same
    code. The effect of this is that you can't "net rpc rights grant" a right,
    for example SeInteractiveLogonRight. A quick test against a W2k12 server
    shows that W2k12 allows this call.
    
    This patch changes the semantics of dcesrv_lsa_AddRemoveAccountRights
    to also allow "rights" to be granted and revoked. At the same
    time, it centralizes the check for validity of user input from
    dcesrv_lsa_EnumAccountsWithUserRight into dcesrc_lsa_valid_AccountRight
    too.
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    
    Autobuild-User(master): Jeremy Allison <jra at samba.org>
    Autobuild-Date(master): Sat Apr 29 09:20:02 CEST 2017 on sn-devel-144

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

Summary of changes:
 source4/rpc_server/lsa/dcesrv_lsa.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source4/rpc_server/lsa/dcesrv_lsa.c b/source4/rpc_server/lsa/dcesrv_lsa.c
index f013d99..3cccd57 100644
--- a/source4/rpc_server/lsa/dcesrv_lsa.c
+++ b/source4/rpc_server/lsa/dcesrv_lsa.c
@@ -99,6 +99,24 @@ struct lsa_trusted_domain_state {
 	struct ldb_dn *trusted_domain_user_dn;
 };
 
+static bool dcesrc_lsa_valid_AccountRight(const char *right)
+{
+	enum sec_privilege priv_id;
+	uint32_t right_bit;
+
+	priv_id = sec_privilege_id(right);
+	if (priv_id != SEC_PRIV_INVALID) {
+		return true;
+	}
+
+	right_bit = sec_right_bit(right);
+	if (right_bit != 0) {
+		return true;
+	}
+
+	return false;
+}
+
 /*
   this is based on the samba3 function make_lsa_object_sd()
   It uses the same logic, but with samba4 helper functions
@@ -2949,12 +2967,10 @@ static NTSTATUS dcesrv_lsa_AddRemoveAccountRights(struct dcesrv_call_state *dce_
 	}
 
 	for (i=0;i<rights->count;i++) {
-		if (sec_privilege_id(rights->names[i].string) == SEC_PRIV_INVALID) {
-			if (sec_right_bit(rights->names[i].string) == 0) {
-				talloc_free(msg);
-				return NT_STATUS_NO_SUCH_PRIVILEGE;
-			}
+		bool ok;
 
+		ok = dcesrc_lsa_valid_AccountRight(rights->names[i].string);
+		if (!ok) {
 			talloc_free(msg);
 			return NT_STATUS_NO_SUCH_PRIVILEGE;
 		}
@@ -3856,6 +3872,7 @@ static NTSTATUS dcesrv_lsa_EnumAccountsWithUserRight(struct dcesrv_call_state *d
 	struct ldb_message **res;
 	const char * const attrs[] = { "objectSid", NULL};
 	const char *privname;
+	bool ok;
 
 	DCESRV_PULL_HANDLE(h, r->in.handle, LSA_HANDLE_POLICY);
 
@@ -3866,7 +3883,9 @@ static NTSTATUS dcesrv_lsa_EnumAccountsWithUserRight(struct dcesrv_call_state *d
 	}
 
 	privname = r->in.name->string;
-	if (sec_privilege_id(privname) == SEC_PRIV_INVALID && sec_right_bit(privname) == 0) {
+
+	ok = dcesrc_lsa_valid_AccountRight(privname);
+	if (!ok) {
 		return NT_STATUS_NO_SUCH_PRIVILEGE;
 	}
 


-- 
Samba Shared Repository