libcli:security:sddl: accept only 8-4-4-4-12 GUIDs

Before we would take strings in a variety of lengths and formats,
which is not what Windows does or [MS-DTYP] says.

This was found by looking at evolved fuzz seeds. Note the 16 and 32
byte sequences in GUID position below:

$ hd $(ls -t seeds/fuzz_sddl_parse/* | head -1)| head
00000000  44 3a 41 52 50 50 50 50  50 28 4f 4c 3b 3b 46 57  |D:ARPPPPP(OL;;FW|
00000010  3b 30 7e ff ff ff ff ff  ff ff 2d 31 38 f5 ff ff  |;0~.......-18...|
00000020  fb 3b 3b 52 43 29 28 4f  44 3b 3b 46 57 3b 3b 3b  |.;;RC)(OD;;FW;;;|
00000030  52 43 29 28 4f 44 3b 3b  46 57 3b 30 30 ff ff ff  |RC)(OD;;FW;00...|
00000040  fb 30 e9 9b 3c cf e6 f5  ff ff fb 3b 3b 52 43 29  |.0..<......;;RC)|
00000050  28 4f 44 3b 3b 46 57 43  52 3b 3b 3b 52 43 29 28  |(OD;;FWCR;;;RC)(|
00000060  4f 44 3b 3b 46 58 47 52  3b 3b 33 43 43 35 38 37  |OD;;FXGR;;3CC587|
00000070  32 35 44 44 44 44 44 44  44 44 44 44 44 44 44 44  |25DDDDDDDDDDDDDD|
00000080  44 44 44 44 44 44 44 44  44 44 3b 52 43 29 28 4f  |DDDDDDDDDD;RC)(O|
00000090  44 3b 3b 46 58 3b 3b 3b  52 43 29 28 4f 44 3b 3b  |D;;FX;;;RC)(OD;;|

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 8e67f90bf865188a7965b74b612d93673b12aeaf..74e0f0dccae0188c1cdddd3c6de14808b1c7726d 100644 (file)
--- a/libcli/security/sddl.c
+++ b/libcli/security/sddl.c
@@ -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;
@@ -417,6 +419,16 @@ static bool sddl_decode_access(const char *str, uint32_t *pmask)
        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
@@ -472,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;
@@ -482,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;

diff --git a/selftest/knownfail.d/sddl b/selftest/knownfail.d/sddl
index 44c018ac8d733e363662300430e922fe31d9f009..3c33617e793cc269ba76bf1673cbca6157d86eea 100644 (file)
--- a/selftest/knownfail.d/sddl
+++ b/selftest/knownfail.d/sddl
@@ -1,6 +1,3 @@
-^samba.tests.sddl.+.SddlShouldFail.test_sddl_should_fail_D:.A;;GA;;0123456789abcdef0123456789abcdef;WD..none
-^samba.tests.sddl.+.SddlShouldFail.test_sddl_should_fail_D:.A;;GA;;0123456789abcdef;WD..none
-^samba.tests.sddl.+.SddlShouldFail.test_sddl_should_fail_D:.A;;GA;;{f30e3bbf-9ff0-11d1-b603-0000f80367c1};WD..none
 ^samba.tests.sddl.+.SddlWindowsFlagsAreDifferent.test_sddl_D:.A;;0x001f01ff;;;WD..A;;0x001f01ff;;;S-1-5-21-11111111-22222222-33333333-1001..A;;0x001f01ff;;;S-1.11522-more-characters.none
 ^samba.tests.sddl.+.SddlWindowsFlagsAreDifferent.test_sddl_D:.A;;FA;;;WD..none
 ^samba.tests.sddl.+.SddlWindowsFlagsAreDifferent.test_sddl_O:S-1-5-21-2212615479-2695158682-2101375468-512G:S-1-5-21-2212615479-2695158682-2101375468-513D:P.A;.482-more-characters.none