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.

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]>

Cheers,
Daniel
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to