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

client/X11: harden xf_cliprdr_parse_server_format_list()

Browse Source 
* Make sure that numFormats has reasonable value

This will help catching errors like writing -1 as an unsigned number
of formats into the serialized stream, or trying to read the property
after someone else erroneosly messed with it, or other similar mistakes
which would result into reading and then sending garbage to the server.

We read the list xf_cliprdr_get_raw_server_formats() from an X window
property. Properties generally cannot be larger than 4 KB and each
format requires at least 5 bytes (most of them are named, though),
which gives us 512-ish limit on the number of formats we can squeeze
into the property.

However, it's hard to find an application that provides more than
20 formats (I've seen like 15 for MS Office apps), thus I believe
we can safely assume than anything that does not fit into a byte
means that we are reading garbage rather than a good format list.

* Check for the end of stream when reading format names

This also prevents reading garbage and getting segmentation faults
and Valgrind warnings when somebody somewhere sometimes forgets to
put a terminating null character where it belongs.

strnlen() and strndup() functions are provided by POSIX.1-2008
which we can reasonably expect to be available in 2016.
This commit is contained in:
ilammy 2016-02-23 00:14:30 +02:00
parent 7bce7ef372
commit 93fc349ce6
1 changed files with 23 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
client/X11/xf_cliprdr.c
View File

@ -44,6 +44,8 @@
#define TAG CLIENT_TAG("x11")
#define MAX_CLIPBOARD_FORMATS 255
struct xf_cliprdr_format
{
Atom atom;
@ -341,6 +343,12 @@ static CLIPRDR_FORMAT* xf_cliprdr_parse_server_format_list(BYTE* data, size_t le
Stream_Read_UINT32(s, *numFormats);
if (*numFormats > MAX_CLIPBOARD_FORMATS)
{
WLog_ERR(TAG, "unexpectedly large number of formats: %u", *numFormats);
goto error;
}
if (!(formats = (CLIPRDR_FORMAT*) calloc(*numFormats, sizeof(CLIPRDR_FORMAT))))
{
WLog_ERR(TAG, "failed to allocate format list");
@ -349,6 +357,9 @@ static CLIPRDR_FORMAT* xf_cliprdr_parse_server_format_list(BYTE* data, size_t le
for (i = 0; i < *numFormats; i++)
{
const char* formatName = NULL;
size_t formatNameLength = 0;
if (Stream_GetRemainingLength(s) < sizeof(UINT32))
{
WLog_ERR(TAG, "unexpected end of serialized format list");
@ -356,8 +367,18 @@ static CLIPRDR_FORMAT* xf_cliprdr_parse_server_format_list(BYTE* data, size_t le
}
Stream_Read_UINT32(s, formats[i].formatId);
formats[i].formatName = strdup((char*) Stream_Pointer(s));
Stream_Seek(s, strlen((char*) Stream_Pointer(s)) + 1);
formatName = (const char*) Stream_Pointer(s);
formatNameLength = strnlen(formatName, Stream_GetRemainingLength(s));
if (formatNameLength == Stream_GetRemainingLength(s))
{
WLog_ERR(TAG, "missing terminating null byte, %zu bytes left to read", formatNameLength);
goto error;
}
formats[i].formatName = strndup(formatName, formatNameLength);
Stream_Seek(s, formatNameLength + 1);
}
Stream_Free(s, FALSE);