[GitHub] [guacamole-server] mike-jumper commented on a change in pull request #275: GUACAMOLE-1059: Check array boundary for sound formats

[GitBox <gi...@apache.org>]

mike-jumper commented on a change in pull request #275:
URL: https://github.com/apache/guacamole-server/pull/275#discussion_r429522227



##########
File path: src/protocols/rdp/channels/rdpsnd/rdpsnd-messages.c
##########
@@ -296,7 +296,7 @@ void guac_rdpsnd_wave_info_handler(guac_rdp_common_svc* svc,
     rdpsnd->next_pdu_is_wave = TRUE;
 
     /* Reset audio stream if format has changed */
-    if (audio != NULL)
+    if (audio != NULL && format < sizeof(rdpsnd->formats))

Review comment:
       Looks good - very straightforward. The only nit I'd like to pick is logging: rather than silently trudge forward, I think we should log that something isn't right, just as in #274. If we encounter a faulty RDP server and this occurs, it would be good to know that it occurred, both for debugging and for reporting upstream.
   
   Lacking that, this sort of failure would no doubt result in JIRA issues / mailing list threads regarding mysterious lack of sound, and it would take some involved digging before things could be established to not be a guac issue.




----------------------------------------------------------------
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:
users@infra.apache.org