From e7e04b7be3f018ad636aba3a36bfc1cd80b9906d Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 13 Apr 2013 11:27:26 -0700 Subject: [PATCH 2/2] integer overflow in XRecordGetContext() [CVE-2013-2063] The nclients and nranges members of the reply are both CARD32 and need to be bounds checked before multiplying by the size of the structs to avoid integer overflow leading to underallocation and writing data from the network past the end of the allocated buffer. Signed-off-by: Alan Coopersmith --- src/XRecord.c | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/src/XRecord.c b/src/XRecord.c index ba628b6..5bbd5ac 100644 --- a/src/XRecord.c +++ b/src/XRecord.c @@ -420,11 +420,9 @@ XRecordGetContext(Display *dpy, XRecordContext context, XExtDisplayInfo *info = find_display (dpy); register xRecordGetContextReq *req; xRecordGetContextReply rep; - int count, i, rn; + unsigned int count, i, rn; xRecordRange xrange; - XRecordRange *ranges = NULL; xRecordClientInfo xclient_inf; - XRecordClientInfo **client_inf, *client_inf_str = NULL; XRecordState *ret; XRecordCheckExtension (dpy, info, 0); @@ -454,13 +452,18 @@ XRecordGetContext(Display *dpy, XRecordContext context, if (count) { - client_inf = (XRecordClientInfo **) Xcalloc(count, sizeof(XRecordClientInfo*)); - ret->client_info = client_inf; - if (client_inf != NULL) { - client_inf_str = (XRecordClientInfo *) Xmalloc(count*sizeof(XRecordClientInfo)); + XRecordClientInfo **client_inf = NULL; + XRecordClientInfo *client_inf_str = NULL; + + if (count < (INT_MAX / sizeof(XRecordClientInfo))) { + client_inf = Xcalloc(count, sizeof(XRecordClientInfo *)); + if (client_inf != NULL) + client_inf_str = Xmalloc(count * sizeof(XRecordClientInfo)); } + ret->client_info = client_inf; if (!client_inf || !client_inf_str) { + free(client_inf); _XEatDataWords (dpy, rep.length); UnlockDisplay(dpy); XRecordFreeState(ret); @@ -476,11 +479,18 @@ XRecordGetContext(Display *dpy, XRecordContext context, if (xclient_inf.nRanges) { - client_inf_str[i].ranges = (XRecordRange**) Xcalloc(xclient_inf.nRanges, sizeof(XRecordRange*)); - if (client_inf_str[i].ranges != NULL) { - ranges = (XRecordRange*) - Xmalloc(xclient_inf.nRanges * sizeof(XRecordRange)); + XRecordRange *ranges = NULL; + + if (xclient_inf.nRanges < (INT_MAX / sizeof(XRecordRange))) { + client_inf_str[i].ranges = + Xcalloc(xclient_inf.nRanges, sizeof(XRecordRange *)); + if (client_inf_str[i].ranges != NULL) + ranges = + Xmalloc(xclient_inf.nRanges * sizeof(XRecordRange)); } + else + client_inf_str[i].ranges = NULL; + if (!client_inf_str[i].ranges || !ranges) { /* XXX eat data */ UnlockDisplay(dpy); -- 1.8.2.3