r14891: fix a bug found by the ibm checker

the problem was that we shift with <<= (privilege-1)

and we called the function with privilege=0

add some checks to catch invalid privilege values
and hide the mask representation in privilege.c

metze
(This used to be commit a69f000324764bcd4cf420f2ecba1aca788258e4)

diff --git a/source4/dsdb/samdb/samdb_privilege.c b/source4/dsdb/samdb/samdb_privilege.c
index 6ca8aa7ae20106252b5e8c4657d823efcd6d8e4d..f2fc43967faee36236c64c227615d706226b13df 100644 (file)
--- a/source4/dsdb/samdb/samdb_privilege.c
+++ b/source4/dsdb/samdb/samdb_privilege.c
@@ -31,16 +31,14 @@
   add privilege bits for one sid to a security_token
 */
 static NTSTATUS samdb_privilege_setup_sid(void *samctx, TALLOC_CTX *mem_ctx,
-                                         const struct dom_sid *sid, 
-                                         uint64_t *mask)
+                                         struct security_token *token,
+                                         const struct dom_sid *sid)
 {
        const char * const attrs[] = { "privilege", NULL };
        struct ldb_message **res = NULL;
        struct ldb_message_element *el;
        int ret, i;
        char *sidstr;
-       
-       *mask = 0;
 
        sidstr = ldap_encode_ndr_dom_sid(mem_ctx, sid);
        NT_STATUS_HAVE_NO_MEMORY(sidstr);
@@ -59,13 +57,13 @@ static NTSTATUS samdb_privilege_setup_sid(void *samctx, TALLOC_CTX *mem_ctx,
 
        for (i=0;i<el->num_values;i++) {
                const char *priv_str = (const char *)el->values[i].data;
-               int privilege = sec_privilege_id(priv_str);
+               enum sec_privilege privilege = sec_privilege_id(priv_str);
                if (privilege == -1) {
                        DEBUG(1,("Unknown privilege '%s' in samdb\n",
                                 priv_str));
                        continue;
                }
-               *mask |= sec_privilege_mask(privilege);
+               sec_privilege_set(token, privilege);
        }
 
        return NT_STATUS_OK;
@@ -103,14 +101,12 @@ _PUBLIC_ NTSTATUS samdb_privilege_setup(struct security_token *token)
        token->privilege_mask = 0;
        
        for (i=0;i<token->num_sids;i++) {
-               uint64_t mask;
-               status = samdb_privilege_setup_sid(samctx, mem_ctx, 
-                                                  token->sids[i], &mask);
+               status = samdb_privilege_setup_sid(samctx, mem_ctx,
+                                                  token, token->sids[i]);
                if (!NT_STATUS_IS_OK(status)) {
                        talloc_free(mem_ctx);
                        return status;
                }
-               token->privilege_mask |= mask;
        }
 
        talloc_free(mem_ctx);

diff --git a/source4/libcli/security/privilege.c b/source4/libcli/security/privilege.c
index 202a418f6ba6283c86a6a9f4556528d29378e24b..f81ff6dccc2ebdca3b53f4ce3d7f0c686aabcda3 100644 (file)
--- a/source4/libcli/security/privilege.c
+++ b/source4/libcli/security/privilege.c
@@ -130,7 +130,7 @@ static const struct {
 /*
   map a privilege id to the wire string constant
 */
-const char *sec_privilege_name(unsigned int privilege)
+const char *sec_privilege_name(enum sec_privilege privilege)
 {
        int i;
        for (i=0;i<ARRAY_SIZE(privilege_names);i++) {
@@ -146,9 +146,12 @@ const char *sec_privilege_name(unsigned int privilege)
   
   TODO: this should use language mappings
 */
-const char *sec_privilege_display_name(int privilege, uint16_t *language)
+const char *sec_privilege_display_name(enum sec_privilege privilege, uint16_t *language)
 {
        int i;
+       if (privilege < 1 || privilege > 64) {
+               return NULL;
+       }
        for (i=0;i<ARRAY_SIZE(privilege_names);i++) {
                if (privilege_names[i].privilege == privilege) {
                        return privilege_names[i].display_name;
@@ -160,12 +163,12 @@ const char *sec_privilege_display_name(int privilege, uint16_t *language)
 /*
   map a privilege name to a privilege id. Return -1 if not found
 */
-int sec_privilege_id(const char *name)
+enum sec_privilege sec_privilege_id(const char *name)
 {
        int i;
        for (i=0;i<ARRAY_SIZE(privilege_names);i++) {
                if (strcasecmp(privilege_names[i].name, name) == 0) {
-                       return (int)privilege_names[i].privilege;
+                       return privilege_names[i].privilege;
                }
        }
        return -1;
@@ -175,9 +178,14 @@ int sec_privilege_id(const char *name)
 /*
   return a privilege mask given a privilege id
 */
-uint64_t sec_privilege_mask(unsigned int privilege)
+static uint64_t sec_privilege_mask(enum sec_privilege privilege)
 {
        uint64_t mask = 1;
+
+       if (privilege < 1 || privilege > 64) {
+               return 0;
+       }
+
        mask <<= (privilege-1);
        return mask;
 }
@@ -186,9 +194,15 @@ uint64_t sec_privilege_mask(unsigned int privilege)
 /*
   return True if a security_token has a particular privilege bit set
 */
-BOOL sec_privilege_check(const struct security_token *token, unsigned int privilege)
+BOOL sec_privilege_check(const struct security_token *token, enum sec_privilege privilege)
 {
-       uint64_t mask = sec_privilege_mask(privilege);
+       uint64_t mask;
+
+       if (privilege < 1 || privilege > 64) {
+               return False;
+       }
+
+       mask = sec_privilege_mask(privilege);
        if (token->privilege_mask & mask) {
                return True;
        }
@@ -198,7 +212,30 @@ BOOL sec_privilege_check(const struct security_token *token, unsigned int privil
 /*
   set a bit in the privilege mask
 */
-void sec_privilege_set(struct security_token *token, unsigned int privilege)
+void sec_privilege_set(struct security_token *token, enum sec_privilege privilege)
 {
+       if (privilege < 1 || privilege > 64) {
+               return;
+       }
        token->privilege_mask |= sec_privilege_mask(privilege);
 }
+
+void sec_privilege_debug(int dbg_lev, const struct security_token *token)
+{
+       DEBUGADD(dbg_lev, (" Privileges (0x%16llX):\n",
+                           (unsigned long long) token->privilege_mask));
+
+       if (token->privilege_mask) {
+               int i = 0;
+               uint_t privilege;
+
+               for (privilege = 1; privilege <= 64; privilege++) {
+                       uint64_t mask = sec_privilege_mask(privilege);
+
+                       if (token->privilege_mask & mask) {
+                               DEBUGADD(dbg_lev, ("  Privilege[%3lu]: %s\n", (unsigned long)i++, 
+                                       sec_privilege_name(privilege)));
+                       }
+               }
+       }
+}

diff --git a/source4/libcli/security/security_token.c b/source4/libcli/security/security_token.c
index 6e380a62add258ce488930b84ef8e8944363be49..80644e1f2dd884d1df4a9b087a19b70344fac301 100644 (file)
--- a/source4/libcli/security/security_token.c
+++ b/source4/libcli/security/security_token.c
@@ -128,7 +128,6 @@ void security_token_debug(int dbg_lev, const struct security_token *token)
 {
        TALLOC_CTX *mem_ctx;
        int i;
-       uint_t privilege;
 
        if (!token) {
                DEBUG(dbg_lev, ("Security token: (NULL)\n"));
@@ -149,21 +148,7 @@ void security_token_debug(int dbg_lev, const struct security_token *token)
                           dom_sid_string(mem_ctx, token->sids[i])));
        }
 
-       DEBUGADD(dbg_lev, (" Privileges (0x%08X%08X):\n",
-                           (uint32_t)((token->privilege_mask & 0xFFFFFFFF00000000LL) >> 32),
-                           (uint32_t)(token->privilege_mask & 0x00000000FFFFFFFFLL)));
-
-       if (token->privilege_mask) {
-               i = 0;
-               for (privilege = 0; privilege < 64; privilege++) {
-                       uint64_t mask = sec_privilege_mask(privilege);
-
-                       if (token->privilege_mask & mask) {
-                               DEBUGADD(dbg_lev, ("  Privilege[%3lu]: %s\n", (unsigned long)i++, 
-                                       sec_privilege_name(privilege)));
-                       }
-               }
-       }
+       sec_privilege_debug(dbg_lev, token);
 
        talloc_free(mem_ctx);
 }