libcli:security: sddl_map_flags rejects trailing nonsense

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/libcli/security/sddl.c b/libcli/security/sddl.c
index cfd625da6baf5b38f0b7c6f1d2a04c37bb3d645f..b115d893e72111a9bb76fc81f6154c3d2bb24048 100644 (file)
--- a/libcli/security/sddl.c
+++ b/libcli/security/sddl.c
@@ -69,18 +69,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 +85,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;
 }
 
 

diff --git a/selftest/knownfail.d/sddl b/selftest/knownfail.d/sddl
index e50cbad2c8d0287ebb2f9956cf43787eb6798bb5..30fa1f85275963d88a36407f39aec7a613f71569 100644 (file)
--- a/selftest/knownfail.d/sddl
+++ b/selftest/knownfail.d/sddl
@@ -7,7 +7,6 @@
 ^samba.tests.sddl.+.SddlWindowsFlagsAreDifferent.test_sddl_D:.A;;0x001f01ff;;;WD..A;;0x001f01ff;;;S-1.+S
 ^samba.tests.sddl.+.SddlWindowsFlagsAreDifferent.test_sddl_D:.A;;FA;;;WD..none
 ^samba.tests.sddl.+.SddlWindowsFlagsAreDifferent.test_sddl_O:S-1-5-21-2212615479-2695158682-210137546.+:
-^samba.tests.sddl.+.SddlWindowsIsLessFussy.test_sddl_should_fail_D:.A;.;GA;;;LG..none
 ^samba.tests.sddl.+.SddlWindowsIsLessFussy.test_sddl_should_fail_D:.A;;.GA;;;LG..none
 ^samba.tests.sddl.+.SddlWindowsIsLessFussy.test_sddl_should_fail_D:.A;;GA;;;WD...none
 ^samba.tests.sddl.+.SddlWindowsIsLessFussy.test_sddl_should_fail_D:.A;;ga;;;LG..none