[PATCH 4/5] Xephyr: integer overflow in XF86DRIOpenConnection()

[Julien Cristau]

On Thu, May 23, 2013 at 09:27:29 -0700, Alan Coopersmith wrote:

> busIdStringLength is a CARD32 and needs to be bounds checked before adding
> one to it to come up with the total size to allocate, to avoid integer
> overflow leading to underallocation and writing data from the network past
> the end of the allocated buffer.
> 
> Reported-by: Ilja Van Sprundel <ivansprundel at ioactive.com>
> Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
> ---
>  hw/kdrive/ephyr/XF86dri.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/kdrive/ephyr/XF86dri.c b/hw/kdrive/ephyr/XF86dri.c
> index 9d742f3..b1f070e 100644
> --- a/hw/kdrive/ephyr/XF86dri.c
> +++ b/hw/kdrive/ephyr/XF86dri.c
> @@ -225,7 +225,11 @@ XF86DRIOpenConnection(Display * dpy, int screen,
>      }
>  
>      if (rep.length) {
> -        if (!(*busIdString = (char *) calloc(rep.busIdStringLength + 1, 1))) {
> +        if (rep.busIdStringLength < INT_MAX)
> +            *busIdString = calloc(rep.busIdStringLength + 1, 1);
> +        else
> +            *busIdString = NULL;
> +        if (*busIdString == NULL) {
>              _XEatData(dpy, ((rep.busIdStringLength + 3) & ~3));
>              UnlockDisplay(dpy);
>              SyncHandle();

We might still overflow in the _XEatData call, but I guess that's not so
much an issue.

Reviewed-by: Julien Cristau <jcristau at debian.org>

Cheers,
Julien