[GitHub] [guacamole-server] khoek commented on pull request #418: GUACAMOLE-1717: Fix RDP cursor use of uninitialized memory

["khoek (via GitHub)" <gi...@apache.org>]

khoek commented on PR #418:
URL: https://github.com/apache/guacamole-server/pull/418#issuecomment-1475922524

   @necouchman I think that the `if` should definitely be removed in any case, because it's doing something wrong---if we just zero the buffer then e.g. in my setup with GRD I won't see any cursor, but with the `if` removed I see the correctly transmitted cursor from the server.
   
   But I agree we might as well zero it, so I pushed a change.
   
   As for testing I've also tried the RDP server which ships with an up to date copy of Windows 11, but that's all. I don't think we can cause a regression because:
   - The only situation where could introduce one is the case where the `if` condition is `false` and we skip the call, hence return the uninitialized weird-looking junk cursor, and
   - so the only way we could cause a regression is by making this *worse*, i.e. the call to `freerdp_image_copy_from_pointer_data()` now crashes the program.
   But the `freerdp_image_copy_from_pointer_data()` function correctly handles the cases where `xorMaskData` or `andMaskData` are `NULL` (in the former case it just errors and returns `false`, and in the latter case the empty and mask is correctly handled).


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org