libcli/security: SDDL decode stops earlier with too many ACEs

For this purpose, "too many" means we know for sure that it won't fit
in packet format, even if all the ACEs are minimum size. This would
fail anyway.

Credit to OSS-Fuzz, who found that 50 thousand ACEs that took more
than 60 seconds to decode. This will now fail after 4096 ACEs which
should be about 150 times faster than 50k (because the realloc loop in
quadratic), so ~0.5 seconds in the fuzz context with sanitisers
enabled. That is still slowish, but SDDL parsing is not a critical
path and without address sanitisers it will be many times faster.

REF: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=62511

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 3b92404634c33a03f361f718088ecbfba3c64c63..d1f770752385b79495f77eb86da4a06a24110940 100644 (file)
--- a/libcli/security/sddl.c
+++ b/libcli/security/sddl.c
@@ -853,6 +853,16 @@ static struct security_acl *sddl_decode_acl(struct security_descriptor *sd,
 
        while (*sddl_copy == '(') {
                bool ok;
+               if (acl->num_aces > UINT16_MAX / 16) {
+                       /*
+                        * We can't fit this many ACEs in a wire ACL
+                        * which has a 16 bit size field (and 16 is
+                        * the minimal size of an ACE with no subauths).
+                        */
+                       talloc_free(acl);
+                       return NULL;
+               }
+
                acl->aces = talloc_realloc(acl, acl->aces, struct security_ace,
                                           acl->num_aces+1);
                if (acl->aces == NULL) {