mirrors/FreeRDP
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #6019 from akallabeth/bound_access_fixes

Browse Source 
Fix issues with boundary access.
This commit is contained in:
Norbert Federa 2020-04-06 13:53:28 +02:00 committed by GitHub
commit c367f65d42
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 420 additions and 380 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
libfreerdp/cache/bitmap.c vendored
View File

@ -281,11 +281,12 @@ rdpBitmapCache* bitmap_cache_new(rdpSettings* settings)
bitmapCache->settings = settings;
bitmapCache->update = ((freerdp*)settings->instance)->update;
bitmapCache->context = bitmapCache->update->context;
bitmapCache->maxCells = settings->BitmapCacheV2NumCells;
bitmapCache->cells = (BITMAP_V2_CELL*)calloc(bitmapCache->maxCells, sizeof(BITMAP_V2_CELL));
bitmapCache->cells =
(BITMAP_V2_CELL*)calloc(settings->BitmapCacheV2NumCells, sizeof(BITMAP_V2_CELL));
if (!bitmapCache->cells)
goto fail;
bitmapCache->maxCells = settings->BitmapCacheV2NumCells;
for (i = 0; i < (int)bitmapCache->maxCells; i++)
{

2
libfreerdp/core/autodetect.c
View File

@ -465,6 +465,8 @@ static BOOL autodetect_recv_bandwidth_measure_results(rdpRdp* rdp, wStream* s,
return FALSE;
WLog_VRB(AUTODETECT_TAG, "received Bandwidth Measure Results PDU");
if (Stream_GetRemainingLength(s) < 8)
return -1;
Stream_Read_UINT32(s, rdp->autodetect->bandwidthMeasureTimeDelta); /* timeDelta (4 bytes) */
Stream_Read_UINT32(s, rdp->autodetect->bandwidthMeasureByteCount); /* byteCount (4 bytes) */

635
libfreerdp/core/capabilities.c
View File

File diff suppressed because it is too large Load Diff

32
libfreerdp/core/gcc.c
View File

@ -519,18 +519,27 @@ BOOL gcc_read_server_data_blocks(wStream* s, rdpMcs* mcs, int length)
while (offset < length)
{
holdp = Stream_Pointer(s);
size_t rest;
wStream sub;
if (!gcc_read_user_data_header(s, &type, &blockLength))
{
WLog_ERR(TAG, "gcc_read_server_data_blocks: gcc_read_user_data_header failed");
return FALSE;
}
holdp = Stream_Pointer(s);
Stream_StaticInit(&sub, holdp, blockLength - 4);
if (!Stream_SafeSeek(s, blockLength - 4))
{
WLog_ERR(TAG, "gcc_read_server_data_blocks: stream too short");
return FALSE;
}
offset += blockLength;
switch (type)
{
case SC_CORE:
if (!gcc_read_server_core_data(s, mcs))
if (!gcc_read_server_core_data(&sub, mcs))
{
WLog_ERR(TAG, "gcc_read_server_data_blocks: gcc_read_server_core_data failed");
return FALSE;
@ -539,7 +548,7 @@ BOOL gcc_read_server_data_blocks(wStream* s, rdpMcs* mcs, int length)
break;
case SC_SECURITY:
if (!gcc_read_server_security_data(s, mcs))
if (!gcc_read_server_security_data(&sub, mcs))
{
WLog_ERR(TAG,
"gcc_read_server_data_blocks: gcc_read_server_security_data failed");
@ -549,7 +558,7 @@ BOOL gcc_read_server_data_blocks(wStream* s, rdpMcs* mcs, int length)
break;
case SC_NET:
if (!gcc_read_server_network_data(s, mcs))
if (!gcc_read_server_network_data(&sub, mcs))
{
WLog_ERR(TAG,
"gcc_read_server_data_blocks: gcc_read_server_network_data failed");
@ -559,7 +568,7 @@ BOOL gcc_read_server_data_blocks(wStream* s, rdpMcs* mcs, int length)
break;
case SC_MCS_MSGCHANNEL:
if (!gcc_read_server_message_channel_data(s, mcs))
if (!gcc_read_server_message_channel_data(&sub, mcs))
{
WLog_ERR(
TAG,
@ -570,7 +579,7 @@ BOOL gcc_read_server_data_blocks(wStream* s, rdpMcs* mcs, int length)
break;
case SC_MULTITRANSPORT:
if (!gcc_read_server_multitransport_channel_data(s, mcs))
if (!gcc_read_server_multitransport_channel_data(&sub, mcs))
{
WLog_ERR(TAG, "gcc_read_server_data_blocks: "
"gcc_read_server_multitransport_channel_data failed");
@ -584,8 +593,13 @@ BOOL gcc_read_server_data_blocks(wStream* s, rdpMcs* mcs, int length)
break;
}
offset += blockLength;
Stream_SetPointer(s, holdp + blockLength);
rest = Stream_GetRemainingLength(&sub);
if (rest > 0)
{
WLog_WARN(
TAG, "gcc_read_server_data_blocks: ignoring %" PRIuz " bytes with type=%" PRIu16 "",
rest, type);
}
}
return TRUE;
@ -610,7 +624,7 @@ BOOL gcc_read_user_data_header(wStream* s, UINT16* type, UINT16* length)
Stream_Read_UINT16(s, *type); /* type */
Stream_Read_UINT16(s, *length); /* length */
if (Stream_GetRemainingLength(s) < (size_t)(*length - 4))
if ((*length < 4) || (Stream_GetRemainingLength(s) < (size_t)(*length - 4)))
return FALSE;
return TRUE;

2
libfreerdp/core/orders.c
View File

@ -2161,7 +2161,7 @@ static CACHE_BITMAP_V3_ORDER* update_read_cache_bitmap_v3_order(rdpUpdate* updat
Stream_Read_UINT16(s, bitmapData->height); /* height (2 bytes) */
Stream_Read_UINT32(s, new_len); /* length (4 bytes) */
if (Stream_GetRemainingLength(s) < new_len)
if ((new_len == 0) || (Stream_GetRemainingLength(s) < new_len))
goto fail;
new_data = (BYTE*)realloc(bitmapData->data, new_len);

6
libfreerdp/core/peer.c
View File

@ -348,7 +348,6 @@ static int peer_recv_tpkt_pdu(freerdp_peer* client, wStream* s)
rdpRdp* rdp;
UINT16 length;
UINT16 pduType;
UINT16 pduLength;
UINT16 pduSource;
UINT16 channelId;
UINT16 securityFlags = 0;
@ -381,7 +380,8 @@ static int peer_recv_tpkt_pdu(freerdp_peer* client, wStream* s)
if (channelId == MCS_GLOBAL_CHANNEL_ID)
{
if (!rdp_read_share_control_header(s, &pduLength, &pduType, &pduSource))
UINT16 pduLength, remain;
if (!rdp_read_share_control_header(s, &pduLength, &remain, &pduType, &pduSource))
return -1;
client->settings->PduSource = pduSource;
@ -403,6 +403,8 @@ static int peer_recv_tpkt_pdu(freerdp_peer* client, wStream* s)
case PDU_TYPE_FLOW_RESPONSE:
case PDU_TYPE_FLOW_STOP:
case PDU_TYPE_FLOW_TEST:
if (!Stream_SafeSeek(s, remain))
return -1;
break;
default:

86
libfreerdp/core/rdp.c
View File

@ -102,7 +102,7 @@ const char* DATA_PDU_TYPE_STRINGS[80] = {
"?" /* 0x41 - 0x46 */
};
static void rdp_read_flow_control_pdu(wStream* s, UINT16* type);
static BOOL rdp_read_flow_control_pdu(wStream* s, UINT16* type, UINT16* channel_id);
static void rdp_write_share_control_header(wStream* s, UINT16 length, UINT16 type,
UINT16 channel_id);
static void rdp_write_share_data_header(wStream* s, UINT16 length, BYTE type, UINT32 share_id);
@ -143,34 +143,51 @@ void rdp_write_security_header(wStream* s, UINT16 flags)
Stream_Write_UINT16(s, 0); /* flagsHi (unused) */
}
BOOL rdp_read_share_control_header(wStream* s, UINT16* length, UINT16* type, UINT16* channel_id)
BOOL rdp_read_share_control_header(wStream* s, UINT16* tpktLength, UINT16* remainingLength,
UINT16* type, UINT16* channel_id)
{
UINT16 len;
if (Stream_GetRemainingLength(s) < 2)
return FALSE;
/* Share Control Header */
Stream_Read_UINT16(s, *length); /* totalLength */
Stream_Read_UINT16(s, len); /* totalLength */
/* If length is 0x8000 then we actually got a flow control PDU that we should ignore
http://msdn.microsoft.com/en-us/library/cc240576.aspx */
if (*length == 0x8000)
if (len == 0x8000)
{
rdp_read_flow_control_pdu(s, type);
if (!rdp_read_flow_control_pdu(s, type, channel_id))
return FALSE;
*channel_id = 0;
*length = 8; /* Flow control PDU is 8 bytes */
if (tpktLength)
*tpktLength = 8; /* Flow control PDU is 8 bytes */
if (remainingLength)
*remainingLength = 0;
return TRUE;
}
if (((size_t)*length - 2) > Stream_GetRemainingLength(s))
if ((len < 4) || ((len - 2) > Stream_GetRemainingLength(s)))
return FALSE;
if (tpktLength)
*tpktLength = len;
Stream_Read_UINT16(s, *type); /* pduType */
*type &= 0x0F; /* type is in the 4 least significant bits */
if (*length > 4)
if (len > 5)
{
Stream_Read_UINT16(s, *channel_id); /* pduSource */
if (remainingLength)
*remainingLength = len - 6;
}
else
{
*channel_id = 0; /* Windows XP can send such short DEACTIVATE_ALL PDUs. */
if (remainingLength)
*remainingLength = len - 4;
}
return TRUE;
}
@ -1094,7 +1111,7 @@ int rdp_recv_out_of_sequence_pdu(rdpRdp* rdp, wStream* s)
UINT16 length;
UINT16 channelId;
if (!rdp_read_share_control_header(s, &length, &type, &channelId))
if (!rdp_read_share_control_header(s, &length, NULL, &type, &channelId))
return -1;
if (type == PDU_TYPE_DATA)
@ -1116,7 +1133,7 @@ int rdp_recv_out_of_sequence_pdu(rdpRdp* rdp, wStream* s)
}
}
void rdp_read_flow_control_pdu(wStream* s, UINT16* type)
BOOL rdp_read_flow_control_pdu(wStream* s, UINT16* type, UINT16* channel_id)
{
/*
* Read flow control PDU - documented in FlowPDU section in T.128
@ -1126,12 +1143,17 @@ void rdp_read_flow_control_pdu(wStream* s, UINT16* type)
* Switched the order of these two fields to match this observation.
*/
UINT8 pduType;
if (!type)
return FALSE;
if (Stream_GetRemainingLength(s) < 6)
return FALSE;
Stream_Read_UINT8(s, pduType); /* pduTypeFlow */
*type = pduType;
Stream_Seek_UINT8(s); /* pad8bits */
Stream_Seek_UINT8(s); /* flowIdentifier */
Stream_Seek_UINT8(s); /* flowNumber */
Stream_Seek_UINT16(s); /* pduSource */
Stream_Seek_UINT8(s); /* pad8bits */
Stream_Seek_UINT8(s); /* flowIdentifier */
Stream_Seek_UINT8(s); /* flowNumber */
Stream_Read_UINT16(s, *channel_id); /* pduSource */
return TRUE;
}
/**
@ -1265,7 +1287,6 @@ static int rdp_recv_tpkt_pdu(rdpRdp* rdp, wStream* s)
int rc = 0;
UINT16 length;
UINT16 pduType;
UINT16 pduLength;
UINT16 pduSource;
UINT16 channelId = 0;
UINT16 securityFlags = 0;
@ -1319,25 +1340,19 @@ static int rdp_recv_tpkt_pdu(rdpRdp* rdp, wStream* s)
{
while (Stream_GetRemainingLength(s) > 3)
{
size_t startheader, endheader, start, end, diff, headerdiff;
wStream sub;
size_t diff;
UINT16 remain;
startheader = Stream_GetPosition(s);
if (!rdp_read_share_control_header(s, &pduLength, &pduType, &pduSource))
if (!rdp_read_share_control_header(s, NULL, &remain, &pduType, &pduSource))
{
WLog_ERR(TAG, "rdp_recv_tpkt_pdu: rdp_read_share_control_header() fail");
return -1;
}
start = endheader = Stream_GetPosition(s);
headerdiff = endheader - startheader;
if (pduLength < headerdiff)
{
WLog_ERR(
TAG,
"rdp_recv_tpkt_pdu: rdp_read_share_control_header() invalid pduLength %" PRIu16,
pduLength);
Stream_StaticInit(&sub, Stream_Pointer(s), remain);
if (!Stream_SafeSeek(s, remain))
return -1;
}
pduLength -= headerdiff;
rdp->settings->PduSource = pduSource;
rdp->inPackets++;
@ -1345,13 +1360,13 @@ static int rdp_recv_tpkt_pdu(rdpRdp* rdp, wStream* s)
switch (pduType)
{
case PDU_TYPE_DATA:
rc = rdp_recv_data_pdu(rdp, s);
rc = rdp_recv_data_pdu(rdp, &sub);
if (rc < 0)
return rc;
break;
case PDU_TYPE_DEACTIVATE_ALL:
if (!rdp_recv_deactivate_all(rdp, s))
if (!rdp_recv_deactivate_all(rdp, &sub))
{
WLog_ERR(TAG, "rdp_recv_tpkt_pdu: rdp_recv_deactivate_all() fail");
return -1;
@ -1360,14 +1375,14 @@ static int rdp_recv_tpkt_pdu(rdpRdp* rdp, wStream* s)
break;
case PDU_TYPE_SERVER_REDIRECTION:
return rdp_recv_enhanced_security_redirection_packet(rdp, s);
return rdp_recv_enhanced_security_redirection_packet(rdp, &sub);
case PDU_TYPE_FLOW_RESPONSE:
case PDU_TYPE_FLOW_STOP:
case PDU_TYPE_FLOW_TEST:
WLog_DBG(TAG, "flow message 0x%04" PRIX16 "", pduType);
/* http://msdn.microsoft.com/en-us/library/cc240576.aspx */
if (!Stream_SafeSeek(s, pduLength))
if (!Stream_SafeSeek(&sub, remain))
return -1;
break;
@ -1376,16 +1391,13 @@ static int rdp_recv_tpkt_pdu(rdpRdp* rdp, wStream* s)
break;
}
end = Stream_GetPosition(s);
diff = end - start;
if (diff != pduLength)
diff = Stream_GetRemainingLength(&sub);
if (diff > 0)
{
WLog_WARN(TAG,
"pduType %s not properly parsed, %" PRIdz
" bytes remaining unhandled. Skipping.",
pdu_type_to_str(pduType), diff);
if (!Stream_SafeSeek(s, pduLength))
return -1;
}
}
}

3
libfreerdp/core/rdp.h
View File

@ -184,7 +184,8 @@ struct rdp_rdp
FREERDP_LOCAL BOOL rdp_read_security_header(wStream* s, UINT16* flags, UINT16* length);
FREERDP_LOCAL void rdp_write_security_header(wStream* s, UINT16 flags);
FREERDP_LOCAL BOOL rdp_read_share_control_header(wStream* s, UINT16* length, UINT16* type,
FREERDP_LOCAL BOOL rdp_read_share_control_header(wStream* s, UINT16* tpktLength,
UINT16* remainingLength, UINT16* type,
UINT16* channel_id);
FREERDP_LOCAL BOOL rdp_read_share_data_header(wStream* s, UINT16* length, BYTE* type,

18
libfreerdp/core/update.c
View File

@ -103,6 +103,9 @@ static BOOL update_read_bitmap_data(rdpUpdate* update, wStream* s, BITMAP_DATA*
{
if (!(bitmapData->flags & NO_BITMAP_COMPRESSION_HDR))
{
if (Stream_GetRemainingLength(s) < 8)
return FALSE;
Stream_Read_UINT16(s,
bitmapData->cbCompFirstRowSize); /* cbCompFirstRowSize (2 bytes) */
Stream_Read_UINT16(s,
@ -287,14 +290,14 @@ fail:
return NULL;
}
static void update_read_synchronize(rdpUpdate* update, wStream* s)
static BOOL update_read_synchronize(rdpUpdate* update, wStream* s)
{
WINPR_UNUSED(update);
Stream_Seek_UINT16(s); /* pad2Octets (2 bytes) */
/**
* The Synchronize Update is an artifact from the
* T.128 protocol and should be ignored.
*/
return Stream_SafeSeek(s, 2); /* pad2Octets (2 bytes) */
/**
* The Synchronize Update is an artifact from the
* T.128 protocol and should be ignored.
*/
}
static BOOL update_read_play_sound(wStream* s, PLAY_SOUND_UPDATE* play_sound)
@ -807,7 +810,8 @@ BOOL update_recv(rdpUpdate* update, wStream* s)
break;
case UPDATE_TYPE_SYNCHRONIZE:
update_read_synchronize(update, s);
if (!update_read_synchronize(update, s))
goto fail;
rc = IFCALLRESULT(TRUE, update->Synchronize, context);
break;

11
libfreerdp/core/window.c
View File

@ -136,9 +136,6 @@ static BOOL update_read_icon_info(wStream* s, ICON_INFO* iconInfo)
Stream_Read_UINT16(s, iconInfo->cbBitsMask); /* cbBitsMask (2 bytes) */
Stream_Read_UINT16(s, iconInfo->cbBitsColor); /* cbBitsColor (2 bytes) */
if (Stream_GetRemainingLength(s) < iconInfo->cbBitsMask + iconInfo->cbBitsColor)
return FALSE;
/* bitsMask */
newBitMask = (BYTE*)realloc(iconInfo->bitsMask, iconInfo->cbBitsMask);
@ -150,6 +147,8 @@ static BOOL update_read_icon_info(wStream* s, ICON_INFO* iconInfo)
}
iconInfo->bitsMask = newBitMask;
if (Stream_GetRemainingLength(s) < iconInfo->cbBitsMask)
return FALSE;
Stream_Read(s, iconInfo->bitsMask, iconInfo->cbBitsMask);
/* colorTable */
@ -184,7 +183,11 @@ static BOOL update_read_icon_info(wStream* s, ICON_INFO* iconInfo)
}
if (iconInfo->colorTable)
{
if (Stream_GetRemainingLength(s) < iconInfo->cbColorTable)
return FALSE;
Stream_Read(s, iconInfo->colorTable, iconInfo->cbColorTable);
}
/* bitsColor */
newBitMask = (BYTE*)realloc(iconInfo->bitsColor, iconInfo->cbBitsColor);
@ -197,6 +200,8 @@ static BOOL update_read_icon_info(wStream* s, ICON_INFO* iconInfo)
}
iconInfo->bitsColor = newBitMask;
if (Stream_GetRemainingLength(s) < iconInfo->cbBitsColor)
return FALSE;
Stream_Read(s, iconInfo->bitsColor, iconInfo->cbBitsColor);
return TRUE;
}