summaryrefslogtreecommitdiff
path: root/record
diff options
context:
space:
mode:
authorRami Ylimäki <rami.ylimaki@vincit.fi>2011-10-04 12:25:26 +0300
committerJeremy Huddleston <jeremyhu@apple.com>2011-11-18 11:26:02 -0800
commitc1bb8f43b9290c2b18a9f0ac59773ff8f1eb974f (patch)
tree8592b42f3c19f75b64ce0e7ebd6061df1dd010a7 /record
parent1f5baa924a5907978f564372265d9830ac72a077 (diff)
record: Prevent out of bounds access when recording a reply.
Any pad bytes in replies are written to the client from a zeroed array. However, record extension tries to incorrectly access the pad bytes from the end of reply data. Signed-off-by: Rami Ylimäki <rami.ylimaki@vincit.fi> Reviewed-by: Erkki Seppälä <erkki.seppala@vincit.fi>
Diffstat (limited to 'record')
-rw-r--r--record/record.c53
1 files changed, 30 insertions, 23 deletions
diff --git a/record/record.c b/record/record.c
index 68311ac8f..db77b64f5 100644
--- a/record/record.c
+++ b/record/record.c
@@ -269,8 +269,9 @@ RecordFlushReplyBuffer(
* device events and EndOfData, pClient is NULL.
* category is the category of the protocol element, as defined
* by the RECORD spec.
- * data is a pointer to the protocol data, and datalen is its length
- * in bytes.
+ * data is a pointer to the protocol data, and datalen - padlen
+ * is its length in bytes.
+ * padlen is the number of pad bytes from a zeroed array.
* futurelen is the number of bytes that will be sent in subsequent
* calls to this function to complete this protocol element.
* In those subsequent calls, futurelen will be -1 to indicate
@@ -290,7 +291,7 @@ RecordFlushReplyBuffer(
*/
static void
RecordAProtocolElement(RecordContextPtr pContext, ClientPtr pClient,
- int category, pointer data, int datalen, int futurelen)
+ int category, pointer data, int datalen, int padlen, int futurelen)
{
CARD32 elemHeaderData[2];
int numElemHeaders = 0;
@@ -398,15 +399,20 @@ RecordAProtocolElement(RecordContextPtr pContext, ClientPtr pClient,
}
if (datalen)
{
+ static char padBuffer[3]; /* as in FlushClient */
memcpy(pContext->replyBuffer + pContext->numBufBytes,
- data, datalen);
- pContext->numBufBytes += datalen;
+ data, datalen - padlen);
+ pContext->numBufBytes += datalen - padlen;
+ memcpy(pContext->replyBuffer + pContext->numBufBytes,
+ padBuffer, padlen);
+ pContext->numBufBytes += padlen;
}
}
else
+ {
RecordFlushReplyBuffer(pContext, (pointer)elemHeaderData,
- numElemHeaders, (pointer)data, datalen);
-
+ numElemHeaders, (pointer)data, datalen - padlen);
+ }
} /* RecordAProtocolElement */
@@ -483,19 +489,19 @@ RecordABigRequest(RecordContextPtr pContext, ClientPtr client, xReq *stuff)
/* record the request header */
bytesLeft = client->req_len << 2;
RecordAProtocolElement(pContext, client, XRecordFromClient,
- (pointer)stuff, SIZEOF(xReq), bytesLeft);
+ (pointer)stuff, SIZEOF(xReq), 0, bytesLeft);
/* reinsert the extended length field that was squished out */
bigLength = client->req_len + bytes_to_int32(sizeof(bigLength));
if (client->swapped)
swapl(&bigLength);
RecordAProtocolElement(pContext, client, XRecordFromClient,
- (pointer)&bigLength, sizeof(bigLength), /* continuation */ -1);
+ (pointer)&bigLength, sizeof(bigLength), 0, /* continuation */ -1);
bytesLeft -= sizeof(bigLength);
/* record the rest of the request after the length */
RecordAProtocolElement(pContext, client, XRecordFromClient,
- (pointer)(stuff + 1), bytesLeft, /* continuation */ -1);
+ (pointer)(stuff + 1), bytesLeft, 0, /* continuation */ -1);
} /* RecordABigRequest */
@@ -542,7 +548,7 @@ RecordARequest(ClientPtr client)
RecordABigRequest(pContext, client, stuff);
else
RecordAProtocolElement(pContext, client, XRecordFromClient,
- (pointer)stuff, client->req_len << 2, 0);
+ (pointer)stuff, client->req_len << 2, 0, 0);
}
else /* extension, check minor opcode */
{
@@ -566,7 +572,7 @@ RecordARequest(ClientPtr client)
else
RecordAProtocolElement(pContext, client,
XRecordFromClient, (pointer)stuff,
- client->req_len << 2, 0);
+ client->req_len << 2, 0, 0);
break;
}
} /* end for each minor op info */
@@ -619,7 +625,8 @@ RecordAReply(CallbackListPtr *pcbl, pointer nulldata, pointer calldata)
if (pContext->continuedReply)
{
RecordAProtocolElement(pContext, client, XRecordFromServer,
- (pointer)pri->replyData, pri->dataLenBytes, /* continuation */ -1);
+ (pointer)pri->replyData, pri->dataLenBytes,
+ pri->padBytes, /* continuation */ -1);
if (!pri->bytesRemaining)
pContext->continuedReply = 0;
}
@@ -629,7 +636,7 @@ RecordAReply(CallbackListPtr *pcbl, pointer nulldata, pointer calldata)
if (majorop <= 127)
{ /* core reply */
RecordAProtocolElement(pContext, client, XRecordFromServer,
- (pointer)pri->replyData, pri->dataLenBytes, pri->bytesRemaining);
+ (pointer)pri->replyData, pri->dataLenBytes, 0, pri->bytesRemaining);
if (pri->bytesRemaining)
pContext->continuedReply = 1;
}
@@ -651,7 +658,7 @@ RecordAReply(CallbackListPtr *pcbl, pointer nulldata, pointer calldata)
{
RecordAProtocolElement(pContext, client,
XRecordFromServer, (pointer)pri->replyData,
- pri->dataLenBytes, pri->bytesRemaining);
+ pri->dataLenBytes, 0, pri->bytesRemaining);
if (pri->bytesRemaining)
pContext->continuedReply = 1;
break;
@@ -723,7 +730,7 @@ RecordADeliveredEventOrError(CallbackListPtr *pcbl, pointer nulldata, pointer ca
}
RecordAProtocolElement(pContext, pClient,
- XRecordFromServer, pEvToRecord, SIZEOF(xEvent), 0);
+ XRecordFromServer, pEvToRecord, SIZEOF(xEvent), 0, 0);
}
} /* end for each event */
} /* end this client is on this context */
@@ -774,7 +781,7 @@ RecordSendProtocolEvents(RecordClientsAndProtocolPtr pRCAP,
}
RecordAProtocolElement(pContext, NULL,
- XRecordFromServer, pEvToRecord, SIZEOF(xEvent), 0);
+ XRecordFromServer, pEvToRecord, SIZEOF(xEvent), 0, 0);
/* make sure device events get flushed in the absence
* of other client activity
*/
@@ -2415,7 +2422,7 @@ ProcRecordEnableContext(ClientPtr client)
assert(numEnabledContexts > 0);
/* send StartOfData */
- RecordAProtocolElement(pContext, NULL, XRecordStartOfData, NULL, 0, 0);
+ RecordAProtocolElement(pContext, NULL, XRecordStartOfData, NULL, 0, 0, 0);
RecordFlushReplyBuffer(pContext, NULL, 0, NULL, 0);
return Success;
} /* ProcRecordEnableContext */
@@ -2446,7 +2453,7 @@ RecordDisableContext(RecordContextPtr pContext)
if (!pContext->pRecordingClient) return;
if (!pContext->pRecordingClient->clientGone)
{
- RecordAProtocolElement(pContext, NULL, XRecordEndOfData, NULL, 0, 0);
+ RecordAProtocolElement(pContext, NULL, XRecordEndOfData, NULL, 0, 0, 0);
RecordFlushReplyBuffer(pContext, NULL, 0, NULL, 0);
/* Re-enable request processing on this connection. */
AttendClient(pContext->pRecordingClient);
@@ -2761,7 +2768,7 @@ RecordConnectionSetupInfo(RecordContextPtr pContext, NewClientInfoRec *pci)
SwapConnSetupPrefix(pci->prefix, (xConnSetupPrefix*)pConnSetup);
SwapConnSetupInfo((char*)pci->setup, (char*)(pConnSetup + prefixsize));
RecordAProtocolElement(pContext, pci->client, XRecordClientStarted,
- (pointer)pConnSetup, prefixsize + restsize, 0);
+ (pointer)pConnSetup, prefixsize + restsize, 0, 0);
free(pConnSetup);
}
else
@@ -2770,9 +2777,9 @@ RecordConnectionSetupInfo(RecordContextPtr pContext, NewClientInfoRec *pci)
* data in two pieces
*/
RecordAProtocolElement(pContext, pci->client, XRecordClientStarted,
- (pointer)pci->prefix, prefixsize, restsize);
+ (pointer)pci->prefix, prefixsize, 0, restsize);
RecordAProtocolElement(pContext, pci->client, XRecordClientStarted,
- (pointer)pci->setup, restsize, /* continuation */ -1);
+ (pointer)pci->setup, restsize, 0, /* continuation */ -1);
}
} /* RecordConnectionSetupInfo */
@@ -2849,7 +2856,7 @@ RecordAClientStateChange(CallbackListPtr *pcbl, pointer nulldata, pointer callda
{
if (pContext->pRecordingClient && pRCAP->clientDied)
RecordAProtocolElement(pContext, pClient,
- XRecordClientDied, NULL, 0, 0);
+ XRecordClientDied, NULL, 0, 0, 0);
RecordDeleteClientFromRCAP(pRCAP, pos);
}
}