If the server sends us garbage (or the client provides it) then it is
possible for the multiplication to overflow (as it is performed on
unsigned 32-bit values) which will result in a false positive failure of
the sanity check. Avoid it by rearranging arithmetics a little.
Keep the multiplication in the error message because we are interested
in the number of bytes in the stream and how it compares to the number
we have expected based on the presumed file count.
The file contents PDUs support 64-bit file sizes and offsets, but
MS-RDPECLIP explicitly says in 2.2.5.3 File Contents Request PDU that
file larger that 4 gigabytes are not supported by the server. It turns
out that the supported size is even lower than that. The server cannot
correctly handle files larger than 2 gigabytes (inclusive). When faced
with such files it correctly retireves the lower part, but fails to
accept any data past that boundary. After receiving a file range reply
the server repeats the file range request with the same offset, and
again, and again, and again, making no progress and blocking the file
transfer indefinitely. This is not the behavior we would like to have.
Microsoft support site acknowledges and documents the issue [1],
suggesting the users to use disk drive redirection instead to transfer
large files. (File transfers via cliprdr are considerably slower than
disk drive redirection so the suggestion makes very much sense.)
However, we would like to avoid the lockdown of the remote session if
the user does attempt to transfer such files so we add a size check.
Putting it into the conversion from FILEDESCRIPTOR to CLIPRDR_FILELIST
is not an ideal place (the clients may not use the common utilities),
but that's good enough currently.
[1]: https://support.microsoft.com/en-us/help/2258090
The format is described in MS-RDPECLIP 2.2.5.2.3 Packed File List
(CLIPRDR_FILELIST). These functions handle conversion between the
on-the-wire data from cliprdr and arrays of FILEDESCRIPTOR structs.
FILETIME handling is a bit wacky, but that's what we currently have.
- fixed invalid, missing or additional arguments
- removed all type casts from arguments
- added missing (void*) typecasts for %p arguments
- use inttypes defines where appropriate
- Added missing ConvertFromUnicode checks
- If ConvertToUnicode allocates memory, guarantee the null termination
similar to ConvertFromUnicode's implementation
- Fixed some TestUnicodeConversion.c CTest return values
- Added some CTests for ConvertFromUnicode and ConvertToUnicode
- Misc code and protocol hardening fixes in the surrounding code regions
that have been touched