From feebf6746374aa04b12e9e3e51313a3a82c03530 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Wed, 23 Nov 2011 00:30:02 -0800 Subject: [PATCH] Limit the number of screens Xvfb will attempt to allocate memory for 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 Reviewed-by: Jamey Sharp --- hw/vfb/InitOutput.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/vfb/InitOutput.c b/hw/vfb/InitOutput.c index 3e5d05100..9a9905d8f 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();