[PATCH] Limit the number of screens Xvfb will attempt to allocate memory for

[Jamey Sharp]

Sure, that makes sense.

Reviewed-by: Jamey Sharp <jamey at minilop.net>

On 11/23/11, Alan Coopersmith <alan.coopersmith at oracle.com> wrote:
> Commit f9e3a2955d2ca7 removing the MAXSCREEN limit left the screen
> number too unlimited, and allowed any positive int for a screen number:
>
> Xvfb :1 -screen 2147483647 1024x1024x8
>
> Fatal server error:
> Not enough memory for screen 2147483647
>
> Found by Parfait 0.3.7:
> Error: Integer overflow (CWE 190)
>    Integer parameter of memory allocation function realloc() may overflow
> due to multiplication with constant value 1112
>         at line 293 of hw/vfb/InitOutput.c in function 'ddxProcessArgument'.
>
> Since the X11 connection setup only has a CARD8 for number of SCREENS,
> limit to 255 screens, which is also low enough to avoid overflow on the
> sizeof(*vfbScreens) * (screenNum + 1) calculation for realloc.
>
> Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
> ---
>  hw/vfb/InitOutput.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/hw/vfb/InitOutput.c b/hw/vfb/InitOutput.c
> index 3e5d051..9a9905d 100644
> --- a/hw/vfb/InitOutput.c
> +++ b/hw/vfb/InitOutput.c
> @@ -280,7 +280,9 @@ ddxProcessArgument(int argc, char *argv[], int i)
>  	int screenNum;
>  	CHECK_FOR_REQUIRED_ARGUMENTS(2);
>  	screenNum = atoi(argv[i+1]);
> -	if (screenNum < 0)
> +	/* The protocol only has a CARD8 for number of screens in the
> +	   connection setup block, so don't allow more than that. */
> +	if ((screenNum < 0) || (screenNum >= 255))
>  	{
>  	    ErrorF("Invalid screen number %d\n", screenNum);
>  	    UseMsg();
> --
> 1.7.3.2
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>