scard: Fix buffer overflow

Even though we can detect that the server buffer is too small to
receive the APDU result we don't prevent the actual copy of this result
to allocated buffer which results in overflow.
This commit is contained in:
Alexander Zakharov 2018-08-30 12:20:15 +03:00
parent 774a657975
commit 758f7b5156
2 changed files with 24 additions and 52 deletions

57
rdpdr.c
View File

@ -357,50 +357,6 @@ rdpdr_send_client_device_list_announce(void)
channel_send(s, rdpdr_channel); channel_send(s, rdpdr_channel);
} }
void
rdpdr_send_scard_io_completion(uint32 device, uint32 id, uint32 status, uint32 result, uint8 * buffer,
uint32 srv_buf_len)
{
int i;
STREAM s;
#ifdef WITH_SCARD
scard_lock(SCARD_LOCK_RDPDR);
#endif
if (result > srv_buf_len) {
/*
* Not enough space has been allocated by server to store the result.
* Send STATUS_BUFFER_TOO_SMALL error as a IoStatus.
*/
result = 0;
status = RD_STATUS_BUFFER_TOO_SMALL;
}
s = channel_init(rdpdr_channel, 20 + result);
out_uint16_le(s, RDPDR_CTYP_CORE);
out_uint16_le(s, PAKID_CORE_DEVICE_IOCOMPLETION);
out_uint32_le(s, device);
out_uint32_le(s, id);
out_uint32_le(s, status);
out_uint32_le(s, result);
if (result)
out_uint8p(s, buffer, result);
s_mark_end(s);
/* JIF */
#ifdef WITH_DEBUG_RDP5
printf("--> rdpdr_send_scard_io_completion\n");
/* hexdump(s->channel_hdr + 8, s->end - s->channel_hdr - 8); */
#endif
channel_send(s, rdpdr_channel);
#ifdef WITH_SCARD
scard_unlock(SCARD_LOCK_RDPDR);
#endif
}
void void
rdpdr_send_completion(uint32 device, uint32 id, uint32 status, uint32 result, uint8 * buffer, rdpdr_send_completion(uint32 device, uint32 id, uint32 status, uint32 result, uint8 * buffer,
uint32 length) uint32 length)
@ -410,6 +366,15 @@ rdpdr_send_completion(uint32 device, uint32 id, uint32 status, uint32 result, ui
#ifdef WITH_SCARD #ifdef WITH_SCARD
scard_lock(SCARD_LOCK_RDPDR); scard_lock(SCARD_LOCK_RDPDR);
#endif #endif
if (status == RD_STATUS_BUFFER_TOO_SMALL) {
/*
* Not enough space has been allocated by server to store the result.
* Send STATUS_BUFFER_TOO_SMALL error as a IoStatus.
*/
result = 0;
length = 0;
}
s = channel_init(rdpdr_channel, 20 + length); s = channel_init(rdpdr_channel, 20 + length);
out_uint16_le(s, RDPDR_CTYP_CORE); out_uint16_le(s, RDPDR_CTYP_CORE);
out_uint16_le(s, PAKID_CORE_DEVICE_IOCOMPLETION); out_uint16_le(s, PAKID_CORE_DEVICE_IOCOMPLETION);
@ -417,7 +382,8 @@ rdpdr_send_completion(uint32 device, uint32 id, uint32 status, uint32 result, ui
out_uint32_le(s, id); out_uint32_le(s, id);
out_uint32_le(s, status); out_uint32_le(s, status);
out_uint32_le(s, result); out_uint32_le(s, result);
out_uint8p(s, buffer, length); if (length)
out_uint8p(s, buffer, length);
s_mark_end(s); s_mark_end(s);
logger(Protocol, Debug, "rdpdr_send_completion()"); logger(Protocol, Debug, "rdpdr_send_completion()");
@ -795,7 +761,6 @@ rdpdr_process_irp(STREAM s)
//out.size = sizeof(buffer); //out.size = sizeof(buffer);
out.size = bytes_out + 0x14; out.size = bytes_out + 0x14;
#ifdef WITH_SCARD #ifdef WITH_SCARD
scardSetInfo(g_epoch, device, id, bytes_out + 0x14); scardSetInfo(g_epoch, device, id, bytes_out + 0x14);
#endif #endif

19
scard.c
View File

@ -1518,7 +1518,7 @@ copyIORequest_ServerToMyPCSC(SERVER_LPSCARD_IO_REQUEST src, MYPCSC_LPSCARD_IO_RE
static DWORD static DWORD
TS_SCardTransmit(STREAM in, STREAM out) TS_SCardTransmit(STREAM in, STREAM out, uint32 srv_buf_len)
{ {
MYPCSC_DWORD rv; MYPCSC_DWORD rv;
SERVER_DWORD map[7], linkedLen; SERVER_DWORD map[7], linkedLen;
@ -1532,6 +1532,7 @@ TS_SCardTransmit(STREAM in, STREAM out)
MYPCSC_DWORD myCbRecvLength; MYPCSC_DWORD myCbRecvLength;
PMEM_HANDLE lcHandle = NULL; PMEM_HANDLE lcHandle = NULL;
in->p += 0x14; in->p += 0x14;
in_uint32_le(in, map[0]); in_uint32_le(in, map[0]);
in->p += 0x04; in->p += 0x04;
@ -1549,6 +1550,9 @@ TS_SCardTransmit(STREAM in, STREAM out)
if (map[0] & INPUT_LINKED) if (map[0] & INPUT_LINKED)
inSkipLinked(in); inSkipLinked(in);
if (srv_buf_len <= cbRecvLength)
cbRecvLength = srv_buf_len;
in->p += 0x04; in->p += 0x04;
in_uint32_le(in, hCard); in_uint32_le(in, hCard);
myHCard = _scard_handle_list_get_pcsc_handle(hCard); myHCard = _scard_handle_list_get_pcsc_handle(hCard);
@ -1647,6 +1651,7 @@ TS_SCardTransmit(STREAM in, STREAM out)
myPioRecvPci, recvBuf, &myCbRecvLength); myPioRecvPci, recvBuf, &myCbRecvLength);
cbRecvLength = myCbRecvLength; cbRecvLength = myCbRecvLength;
if (pioRecvPci) if (pioRecvPci)
{ {
/* /*
@ -2253,7 +2258,7 @@ TS_SCardAccessStartedEvent(STREAM in, STREAM out)
static RD_NTSTATUS static RD_NTSTATUS
scard_device_control(RD_NTHANDLE handle, uint32 request, STREAM in, STREAM out) scard_device_control(RD_NTHANDLE handle, uint32 request, STREAM in, STREAM out, uint32 srv_buf_len)
{ {
UNUSED(handle); UNUSED(handle);
SERVER_DWORD Result = 0x00000000; SERVER_DWORD Result = 0x00000000;
@ -2359,7 +2364,7 @@ scard_device_control(RD_NTHANDLE handle, uint32 request, STREAM in, STREAM out)
/* ScardTransmit */ /* ScardTransmit */
case SC_TRANSMIT: case SC_TRANSMIT:
{ {
Result = (SERVER_DWORD) TS_SCardTransmit(in, out); Result = (SERVER_DWORD) TS_SCardTransmit(in, out, srv_buf_len);
break; break;
} }
/* SCardControl */ /* SCardControl */
@ -2425,6 +2430,8 @@ scard_device_control(RD_NTHANDLE handle, uint32 request, STREAM in, STREAM out)
out_uint8s(out, addToEnd); out_uint8s(out, addToEnd);
} }
if (Result == SCARD_E_INSUFFICIENT_BUFFER) return RD_STATUS_BUFFER_TOO_SMALL;
return RD_STATUS_SUCCESS; return RD_STATUS_SUCCESS;
} }
@ -2565,16 +2572,16 @@ SC_getNextInQueue()
static void static void
SC_deviceControl(PSCThreadData data) SC_deviceControl(PSCThreadData data)
{ {
RD_NTSTATUS status;
size_t buffer_len = 0; size_t buffer_len = 0;
scard_device_control(data->handle, data->request, data->in, data->out); status = scard_device_control(data->handle, data->request, data->in, data->out, data->srv_buf_len);
buffer_len = (size_t) data->out->p - (size_t) data->out->data; buffer_len = (size_t) data->out->p - (size_t) data->out->data;
/* if iorequest belongs to another epoch, don't send response /* if iorequest belongs to another epoch, don't send response
back to server due to it's considered as abandoned. back to server due to it's considered as abandoned.
*/ */
if (data->epoch == curEpoch) if (data->epoch == curEpoch)
rdpdr_send_scard_io_completion(data->device, data->id, 0, buffer_len, data->out->data, rdpdr_send_completion(data->device, data->id, status, buffer_len, data->out->data, buffer_len);
data->srv_buf_len);
SC_destroyThreadData(data); SC_destroyThreadData(data);
} }