mirror of
https://github.com/openbsd/xenocara.git
synced 2025-12-11 11:49:01 +00:00
Out of boundary access and endless loop in libXtst
A lack of range checks in libXtst allows out of boundary accesses. The checks have to be done in-place here, because it cannot be done without in-depth knowledge of the read data. If XRecordStartOfData, XRecordEndOfData, or XRecordClientDied without a client sequence have attached data, an endless loop would occur. The do-while-loop continues until the current index reaches the end. But in these cases, the current index would not be incremented, leading to an endless processing. From Tobias Stoeckmann / X.Org security advisory Oct 4, 2016
This commit is contained in:
@@ -760,15 +760,23 @@ parse_reply_call_callback(
|
|||||||
switch (rep->category) {
|
switch (rep->category) {
|
||||||
case XRecordFromServer:
|
case XRecordFromServer:
|
||||||
if (rep->elementHeader&XRecordFromServerTime) {
|
if (rep->elementHeader&XRecordFromServerTime) {
|
||||||
|
if (current_index + 4 > rep->length << 2)
|
||||||
|
return Error;
|
||||||
EXTRACT_CARD32(rep->clientSwapped,
|
EXTRACT_CARD32(rep->clientSwapped,
|
||||||
reply->buf+current_index,
|
reply->buf+current_index,
|
||||||
data->server_time);
|
data->server_time);
|
||||||
current_index += 4;
|
current_index += 4;
|
||||||
}
|
}
|
||||||
|
if (current_index + 1 > rep->length << 2)
|
||||||
|
return Error;
|
||||||
switch (reply->buf[current_index]) {
|
switch (reply->buf[current_index]) {
|
||||||
case X_Reply: /* reply */
|
case X_Reply: /* reply */
|
||||||
|
if (current_index + 8 > rep->length << 2)
|
||||||
|
return Error;
|
||||||
EXTRACT_CARD32(rep->clientSwapped,
|
EXTRACT_CARD32(rep->clientSwapped,
|
||||||
reply->buf+current_index+4, datum_bytes);
|
reply->buf+current_index+4, datum_bytes);
|
||||||
|
if (datum_bytes < 0 || datum_bytes > ((INT_MAX >> 2) - 8))
|
||||||
|
return Error;
|
||||||
datum_bytes = (datum_bytes+8) << 2;
|
datum_bytes = (datum_bytes+8) << 2;
|
||||||
break;
|
break;
|
||||||
default: /* error or event */
|
default: /* error or event */
|
||||||
@@ -777,52 +785,73 @@ parse_reply_call_callback(
|
|||||||
break;
|
break;
|
||||||
case XRecordFromClient:
|
case XRecordFromClient:
|
||||||
if (rep->elementHeader&XRecordFromClientTime) {
|
if (rep->elementHeader&XRecordFromClientTime) {
|
||||||
|
if (current_index + 4 > rep->length << 2)
|
||||||
|
return Error;
|
||||||
EXTRACT_CARD32(rep->clientSwapped,
|
EXTRACT_CARD32(rep->clientSwapped,
|
||||||
reply->buf+current_index,
|
reply->buf+current_index,
|
||||||
data->server_time);
|
data->server_time);
|
||||||
current_index += 4;
|
current_index += 4;
|
||||||
}
|
}
|
||||||
if (rep->elementHeader&XRecordFromClientSequence) {
|
if (rep->elementHeader&XRecordFromClientSequence) {
|
||||||
|
if (current_index + 4 > rep->length << 2)
|
||||||
|
return Error;
|
||||||
EXTRACT_CARD32(rep->clientSwapped,
|
EXTRACT_CARD32(rep->clientSwapped,
|
||||||
reply->buf+current_index,
|
reply->buf+current_index,
|
||||||
data->client_seq);
|
data->client_seq);
|
||||||
current_index += 4;
|
current_index += 4;
|
||||||
}
|
}
|
||||||
|
if (current_index + 4 > rep->length<<2)
|
||||||
|
return Error;
|
||||||
if (reply->buf[current_index+2] == 0
|
if (reply->buf[current_index+2] == 0
|
||||||
&& reply->buf[current_index+3] == 0) /* needn't swap 0 */
|
&& reply->buf[current_index+3] == 0) /* needn't swap 0 */
|
||||||
{ /* BIG-REQUESTS */
|
{ /* BIG-REQUESTS */
|
||||||
|
if (current_index + 8 > rep->length << 2)
|
||||||
|
return Error;
|
||||||
EXTRACT_CARD32(rep->clientSwapped,
|
EXTRACT_CARD32(rep->clientSwapped,
|
||||||
reply->buf+current_index+4, datum_bytes);
|
reply->buf+current_index+4, datum_bytes);
|
||||||
} else {
|
} else {
|
||||||
EXTRACT_CARD16(rep->clientSwapped,
|
EXTRACT_CARD16(rep->clientSwapped,
|
||||||
reply->buf+current_index+2, datum_bytes);
|
reply->buf+current_index+2, datum_bytes);
|
||||||
}
|
}
|
||||||
|
if (datum_bytes < 0 || datum_bytes > INT_MAX >> 2)
|
||||||
|
return Error;
|
||||||
datum_bytes <<= 2;
|
datum_bytes <<= 2;
|
||||||
break;
|
break;
|
||||||
case XRecordClientStarted:
|
case XRecordClientStarted:
|
||||||
|
if (current_index + 8 > rep->length << 2)
|
||||||
|
return Error;
|
||||||
EXTRACT_CARD16(rep->clientSwapped,
|
EXTRACT_CARD16(rep->clientSwapped,
|
||||||
reply->buf+current_index+6, datum_bytes);
|
reply->buf+current_index+6, datum_bytes);
|
||||||
datum_bytes = (datum_bytes+2) << 2;
|
datum_bytes = (datum_bytes+2) << 2;
|
||||||
break;
|
break;
|
||||||
case XRecordClientDied:
|
case XRecordClientDied:
|
||||||
if (rep->elementHeader&XRecordFromClientSequence) {
|
if (rep->elementHeader&XRecordFromClientSequence) {
|
||||||
|
if (current_index + 4 > rep->length << 2)
|
||||||
|
return Error;
|
||||||
EXTRACT_CARD32(rep->clientSwapped,
|
EXTRACT_CARD32(rep->clientSwapped,
|
||||||
reply->buf+current_index,
|
reply->buf+current_index,
|
||||||
data->client_seq);
|
data->client_seq);
|
||||||
current_index += 4;
|
current_index += 4;
|
||||||
}
|
} else if (current_index < rep->length << 2)
|
||||||
/* fall through */
|
return Error;
|
||||||
|
datum_bytes = 0;
|
||||||
|
break;
|
||||||
case XRecordStartOfData:
|
case XRecordStartOfData:
|
||||||
case XRecordEndOfData:
|
case XRecordEndOfData:
|
||||||
|
if (current_index < rep->length << 2)
|
||||||
|
return Error;
|
||||||
datum_bytes = 0;
|
datum_bytes = 0;
|
||||||
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (datum_bytes > 0) {
|
if (datum_bytes > 0) {
|
||||||
if (current_index + datum_bytes > rep->length << 2)
|
if (INT_MAX - datum_bytes < (rep->length << 2) - current_index) {
|
||||||
fprintf(stderr,
|
fprintf(stderr,
|
||||||
"XRecord: %lu-byte reply claims %d-byte element (seq %lu)\n",
|
"XRecord: %lu-byte reply claims %d-byte element (seq %lu)\n",
|
||||||
(long)rep->length << 2, current_index + datum_bytes,
|
(unsigned long)rep->length << 2, current_index + datum_bytes,
|
||||||
dpy->last_request_read);
|
dpy->last_request_read);
|
||||||
|
return Error;
|
||||||
|
}
|
||||||
/*
|
/*
|
||||||
* This assignment (and indeed the whole buffer sharing
|
* This assignment (and indeed the whole buffer sharing
|
||||||
* scheme) assumes arbitrary 4-byte boundaries are
|
* scheme) assumes arbitrary 4-byte boundaries are
|
||||||
@@ -874,6 +903,12 @@ XRecordEnableContext(Display *dpy, XRecordContext context,
|
|||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (rep.length > INT_MAX >> 2) {
|
||||||
|
UnlockDisplay(dpy);
|
||||||
|
SyncHandle();
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
|
||||||
if (rep.length > 0) {
|
if (rep.length > 0) {
|
||||||
reply = alloc_reply_buffer(info, rep.length<<2);
|
reply = alloc_reply_buffer(info, rep.length<<2);
|
||||||
if (!reply) {
|
if (!reply) {
|
||||||
|
|||||||
Reference in New Issue
Block a user