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>

1 files changed, 17 insertions, 6 deletions

diff --git a/libcli/security/sddl.c b/libcli/security/sddl.c
index 8e67f90bf86..74e0f0dccae 100644
--- 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;