mike-jumper commented on a change in pull request #274:
URL: https://github.com/apache/guacamole-server/pull/274#discussion_r419746745
##########
File path: src/protocols/rdp/channels/rdpsnd/rdpsnd.c
##########
@@ -48,6 +52,10 @@ void guac_rdpsnd_process_receive(guac_rdp_common_svc* svc,
guac_rdpsnd_wave_handler(svc, input_stream, &header);
return;
}
+
+ /* Check body size */
+ if (Stream_GetRemainingLength(input_stream) < header.body_size)
Review comment:
Well, the [documentation for RDPSND regarding the WaveInfo
PDU](https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpea/c53cd81c-0d7f-4e68-8b95-1c1da68dbaac
) pretty specifically states that the body size does not necessarily
indicate the size of the remainder of the PDU:
> **Header (4 bytes):** An [RDPSND PDU Header (section
2.2.1)](https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpea/863d154b-dbe0-4781-979d-fa48daed721e).
The **msgType** field of the RDPSND PDU Header MUST be set to SNDC_WAVE
(0x02). The **BodySize** field of the RDPSND PDU Header is the size of the
WaveInfo PDU plus the size of the data field of the [Wave
PDU](https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpea/81841793-f5d3-4305-aa22-ddcbd81a96b5)
that immediately follows this packet minus the size of the Header.
Given this, I don't believe it is correct to check `body_size` in this way,
and I'm not sure why it would be working. The above indicates to me that
`body_size` would always be larger than `Stream_GetRemainingLength()` for a
WaveInfo PDU .
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
