On 04/17/15 04:59 AM, Daniel Stone wrote:
Hi,
On 17 April 2015 at 02:49, Alan Coopersmith <[email protected]> wrote:
diff --git a/xkb/xkbActions.c b/xkb/xkbActions.c
index 2a196f1..9dd1cbd 100644
--- a/xkb/xkbActions.c
+++ b/xkb/xkbActions.c
@@ -1103,8 +1103,8 @@ _XkbNextFreeFilter(XkbSrvInfoPtr xkbi)
}
}
xkbi->szFilters *= 2;
- xkbi->filters = realloc(xkbi->filters,
- xkbi->szFilters * sizeof(XkbFilterRec));
+ xkbi->filters = reallocarray(xkbi->filters,
+ xkbi->szFilters, sizeof(XkbFilterRec));
/* 6/21/93 (ef) -- XXX! deal with allocation failure */
memset(&xkbi->filters[xkbi->szFilters / 2], 0,
(xkbi->szFilters / 2) * sizeof(XkbFilterRec));
This stuck out as being potentially unsafe, but seems to just be a bug
(in that filters disappear) rather than a security vulnerability,
because the access is always bounded by szFilters. I can see it being
an issue in other similar codepaths though, where the pattern is to
have szFoo (size of list) and numFoo (actual elements in the list). In
the case where we naïvely increase the list and szFoo overflows, then
anything using numFoo to iterate over the list suddenly becomes
unsafe.
Yeah, the XKB code could always use more work, but I'll leave that for
someone else to tackle later. The inconsistent usage of the various
models for tracking array sizes drives me crazy too:
http://cgit.freedesktop.org/xorg/proto/kbproto/commit/?id=95ee49d90c28b15a3c3be54a233368fc69f3531a
My brain started leaking out my face halfway through reviewing this
diff though, and auditing all sz/num users would be a bit of a
nightmare. So, on the grounds that this makes things better rather
than in any way worse:
Acked-by: Daniel Stone <[email protected]>
Thanks.
--
-Alan Coopersmith- [email protected]
Oracle Solaris Engineering - http://blogs.oracle.com/alanc
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
