From: Douglas Bagnall Date: Tue, 12 Dec 2023 21:57:41 +0000 (+1300) Subject: libcli/security: SDDL decode stops earlier with too many ACEs X-Git-Tag: talloc-2.4.2~338 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e004a5a444f9760ff305154cb0c3f1fe1800e8af;p=thirdparty%2Fsamba.git 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 Reviewed-by: Andrew Bartlett --- diff --git a/libcli/security/sddl.c b/libcli/security/sddl.c index 3b92404634c..d1f77075238 100644 --- 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) {