s3/utils: value for ace_flags value "FA" is incorrect
[samba.git] / libcli / security / sddl.c
index 6cc4e2d1f3635db348d3ebf9cec8e72d960cddce..ee024b2b0d7148cf0228e22658e7019c6f840db3 100644 (file)
@@ -25,6 +25,8 @@
 #include "librpc/gen_ndr/ndr_misc.h"
 #include "lib/util/smb_strtox.h"
 #include "system/locale.h"
+#include "lib/util/util_str_hex.h"
+
 
 struct sddl_transition_state {
        const struct dom_sid *machine_sid;
@@ -69,18 +71,14 @@ static bool sddl_map_flags(const struct flag_map *map, const char *str,
                *plen = 0;
        }
        *pflags = 0;
-       while (str[0] && isupper(str[0])) {
+       while (str[0] != '\0' && isupper((unsigned char)str[0])) {
                size_t len;
                uint32_t flags;
                bool found;
 
                found = sddl_map_flag(map, str, &len, &flags);
                if (!found) {
-                        if (unknown_flag_is_part_of_next_thing) {
-                                return true;
-                        }
-                       DEBUG(1, ("Unknown flag - %s in %s\n", str, str0));
-                       return false;
+                       break;
                }
 
                *pflags |= flags;
@@ -89,7 +87,19 @@ static bool sddl_map_flags(const struct flag_map *map, const char *str,
                }
                str += len;
        }
-       return true;
+       /*
+        * For ACL flags, unknown_flag_is_part_of_next_thing is set,
+        * and we expect some more stuff that isn't flags.
+        *
+         * For ACE flags, unknown_flag_is_part_of_next_thing is unset,
+         * and the flags have been tokenised into their own little
+         * string. We don't expect anything here, even whitespace.
+         */
+        if (*str == '\0' || unknown_flag_is_part_of_next_thing) {
+                return true;
+        }
+       DBG_WARNING("Unknown flag - '%s' in '%s'\n", str, str0);
+        return false;
 }
 
 
@@ -316,7 +326,7 @@ static const struct flag_map ace_access_mask[] = {
 };
 
 static const struct flag_map decode_ace_access_mask[] = {
-       { "FA", FILE_ALL_ACCESS },
+       { "FA", FILE_GENERIC_ALL },
        { "FR", FILE_GENERIC_READ },
        { "FW", FILE_GENERIC_WRITE },
        { "FX", FILE_GENERIC_EXECUTE },
@@ -326,6 +336,7 @@ static const struct flag_map decode_ace_access_mask[] = {
 static bool sddl_decode_access(const char *str, uint32_t *pmask)
 {
        const char *str0 = str;
+        char *end = NULL;
        uint32_t mask = 0;
        unsigned long long numeric_mask;
        int err;
@@ -336,21 +347,59 @@ static bool sddl_decode_access(const char *str, uint32_t *pmask)
         * per MS-DTYP and Windows behaviour, octal and decimal numbers are
         * also accepted.
         *
-        * Windows will truncate overflowing numbers at 0xffffffff,
-        * while we will wrap around, using only the lower 32 bits.
+        * Windows has two behaviours we choose not to replicate:
+        *
+        * 1. numbers exceeding 0xffffffff are truncated at that point,
+        *    turning on all access flags.
+        *
+        * 2. negative numbers are accepted, so e.g. -2 becomes 0xfffffffe.
         */
-       numeric_mask = smb_strtoull(str, NULL, 0, &err, SMB_STR_STANDARD);
+       numeric_mask = smb_strtoull(str, &end, 0, &err, SMB_STR_STANDARD);
        if (err == 0) {
+               if (numeric_mask > UINT32_MAX) {
+                       DBG_WARNING("Bad numeric flag value - %llu in %s\n",
+                                   numeric_mask, str0);
+                       return false;
+               }
+               if (end - str > sizeof("037777777777")) {
+                       /* here's the tricky thing: if a number is big
+                        * enough to overflow the uint64, it might end
+                        * up small enough to fit in the uint32, and
+                        * we'd miss that it overflowed. So we count
+                        * the digits -- any more than 12 (for
+                        * "037777777777") is too long for 32 bits,
+                        * and the shortest 64-bit wrapping string is
+                        * 19 (for "0x1" + 16 zeros).
+                        */
+                       DBG_WARNING("Bad numeric flag value in '%s'\n", str0);
+                       return false;
+               }
+               if (*end != '\0') {
+                       DBG_WARNING("Bad characters in '%s'\n", str0);
+                       return false;
+               }
                *pmask = numeric_mask;
                return true;
        }
-       /* It's not a number, so we'll look for flags */
+       /* It's not a positive number, so we'll look for flags */
 
-       while ((str[0] != '\0') && isupper(str[0])) {
+       while ((str[0] != '\0') &&
+              (isupper((unsigned char)str[0]) || str[0] == ' ')) {
                uint32_t flags = 0;
                size_t len = 0;
                bool found;
-
+               while (str[0] == ' ') {
+                       /*
+                        * Following Windows we accept spaces between flags
+                        * but not after flags. Not tabs, though, never tabs.
+                        */
+                       str++;
+                       if (str[0] == '\0') {
+                               DBG_WARNING("trailing whitespace in flags "
+                                           "- '%s'\n", str0);
+                               return false;
+                       }
+               }
                found = sddl_map_flag(
                        ace_access_mask, str, &len, &flags);
                found |= sddl_map_flag(
@@ -362,11 +411,24 @@ static bool sddl_decode_access(const char *str, uint32_t *pmask)
                mask |= flags;
                str += len;
        }
-
+        if (*str != '\0') {
+               DBG_WARNING("Bad characters in '%s'\n", str0);
+                return false;
+        }
        *pmask = mask;
        return true;
 }
 
+
+static bool sddl_decode_guid(const char *str, struct GUID *guid)
+{
+        if (strlen(str) != 36) {
+                return false;
+        }
+        return parse_guid_string(str, guid);
+}
+
+
 /*
   decode an ACE
   return true on success, false on failure
@@ -381,6 +443,7 @@ static bool sddl_decode_ace(TALLOC_CTX *mem_ctx, struct security_ace *ace, char
        uint32_t v;
        struct dom_sid *sid;
        bool ok;
+       size_t len;
 
        ZERO_STRUCTP(ace);
 
@@ -395,9 +458,16 @@ static bool sddl_decode_ace(TALLOC_CTX *mem_ctx, struct security_ace *ace, char
        }
 
        /* parse ace type */
-       if (!sddl_map_flags(ace_types, tok[0], &v, NULL, false)) {
+       ok = sddl_map_flag(ace_types, tok[0], &len, &v);
+       if (!ok) {
+               DBG_WARNING("Unknown ACE type - %s\n", tok[0]);
+               return false;
+       }
+       if (tok[0][len] != '\0') {
+               DBG_WARNING("Garbage after ACE type - %s\n", tok[0]);
                return false;
        }
+
        ace->type = v;
 
        /* ace flags */
@@ -414,9 +484,8 @@ static bool sddl_decode_ace(TALLOC_CTX *mem_ctx, struct security_ace *ace, char
 
        /* object */
        if (tok[3][0] != 0) {
-               NTSTATUS status = GUID_from_string(tok[3],
-                                                  &ace->object.object.type.type);
-               if (!NT_STATUS_IS_OK(status)) {
+               ok = sddl_decode_guid(tok[3], &ace->object.object.type.type);
+               if (!ok) {
                        return false;
                }
                ace->object.object.flags |= SEC_ACE_OBJECT_TYPE_PRESENT;
@@ -424,9 +493,9 @@ static bool sddl_decode_ace(TALLOC_CTX *mem_ctx, struct security_ace *ace, char
 
        /* inherit object */
        if (tok[4][0] != 0) {
-               NTSTATUS status = GUID_from_string(tok[4],
-                                                  &ace->object.object.inherited_type.inherited_type);
-               if (!NT_STATUS_IS_OK(status)) {
+               ok = sddl_decode_guid(tok[4],
+                                     &ace->object.object.inherited_type.inherited_type);
+               if (!ok) {
                        return false;
                }
                ace->object.object.flags |= SEC_ACE_INHERITED_OBJECT_TYPE_PRESENT;
@@ -440,7 +509,9 @@ static bool sddl_decode_ace(TALLOC_CTX *mem_ctx, struct security_ace *ace, char
        }
        ace->trustee = *sid;
        talloc_free(sid);
-
+       if (*s != '\0') {
+               return false;
+       }
        return true;
 }
 
@@ -705,7 +776,7 @@ static char *sddl_transition_encode_ace(TALLOC_CTX *mem_ctx, const struct securi
        sddl_mask = sddl_flags_to_string(tmp_ctx, ace_access_mask,
                                         ace->access_mask, true);
        if (sddl_mask == NULL) {
-               sddl_mask = talloc_asprintf(tmp_ctx, "0x%08x",
+               sddl_mask = talloc_asprintf(tmp_ctx, "0x%x",
                                             ace->access_mask);
                if (sddl_mask == NULL) {
                        goto failed;