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

Fix protocol violation when uploading large ICON

Browse Source 
When uploading large ICON (96x96), we end up growing the stream
mid-update. Stream_EnsureCapacity end up reallocating the stream
with a larger capacity to accomodate the large ICON size, but in
doing so, also updating the sealed length for the data currently in
the stream. This breaks the assumption between update_begin_paint
and update_end_paint where the sealed lenght is used to keep track
of the location where we need to update the orders counts after
we're done accumulating update. As a result of the growth and lost
of that location, the number of orders is written to the wrong
location and the resulting stream is invalid which result in a
protocol violation and a connection drop.

The current fix uses a new offsetOrder in the update object to
keep track of where update_end_paint needs to write the number
of orders contained. I think a better fix would be for
Stream_EnsureCapacity to preserve the sealead length of the
stream on growth, but this has a much more significant impact and
careful analysis needs to be done to ensure this doesn't violate
other assumption. Need to follow up with FreeRDP developer to get
their take on this one.
This commit is contained in:
Steve Pronovost 2021-10-16 11:39:34 -07:00 committed by akallabeth
parent dba2a7e4a6
commit 49d9e61884
2 changed files with 4 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
include/freerdp/update.h
View File

@ -254,6 +254,7 @@ struct rdp_update
wStream* us;
UINT16 numberOrders;
UINT16 offsetOrders; /* the offset to patch numberOrders in the stream */
BOOL combineUpdates;
rdpBounds currentBounds;
rdpBounds previousBounds;

6
libfreerdp/core/update.c
View File

@ -931,6 +931,7 @@ static BOOL _update_begin_paint(rdpContext* context)
return FALSE;
Stream_SealLength(s);
Stream_GetLength(s, update->offsetOrders);
Stream_Seek(s, 2); /* numberOrders (2 bytes) */
update->combineUpdates = TRUE;
update->numberOrders = 0;
@ -941,16 +942,14 @@ static BOOL _update_begin_paint(rdpContext* context)
static BOOL _update_end_paint(rdpContext* context)
{
wStream* s;
int headerLength;
rdpUpdate* update = context->update;
if (!update->us)
return FALSE;
s = update->us;
headerLength = Stream_Length(s);
Stream_SealLength(s);
Stream_SetPosition(s, headerLength);
Stream_SetPosition(s, update->offsetOrders);
Stream_Write_UINT16(s, update->numberOrders); /* numberOrders (2 bytes) */
Stream_SetPosition(s, Stream_Length(s));
@ -962,6 +961,7 @@ static BOOL _update_end_paint(rdpContext* context)
update->combineUpdates = FALSE;
update->numberOrders = 0;
update->offsetOrders = 0;
update->us = NULL;
Stream_Free(s, TRUE);
return TRUE;