[Xen-changelog] [qemu-xen master] ui: avoid sign extension using client width/height

[patchbot@xxxxxxx]

commit 64653b7fbe616b7159fd1e5be2f04f30e048dda4
Author:     Daniel P. Berrange <berrange@xxxxxxxxxx>
AuthorDate: Thu Jan 18 15:52:54 2018 +0000
Commit:     Michael Roth <mdroth@xxxxxxxxxxxxxxxxxx>
CommitDate: Mon Feb 12 18:34:43 2018 -0600

    ui: avoid sign extension using client width/height
    
    Pixman returns a signed int for the image width/height, but the VNC
    protocol only permits a unsigned int16. Effective framebuffer size
    is determined by the guest, limited by the video RAM size, so the
    dimensions are unlikely to exceed the range of an unsigned int16,
    but this is not currently validated.
    
    With the current use of 'int' for client width/height, the calculation
    of offsets in vnc_update_throttle_offset() suffers from integer size
    promotion and sign extension, causing coverity warnings
    
    *** CID 1385147:  Integer handling issues  (SIGN_EXTENSION)
    /ui/vnc.c: 979 in vnc_update_throttle_offset()
    973      * than that the client would already suffering awful audio
    974      * glitches, so dropping samples is no worse really).
    975      */
    976     static void vnc_update_throttle_offset(VncState *vs)
    977     {
    978         size_t offset =
    >>>     CID 1385147:  Integer handling issues  (SIGN_EXTENSION)
    >>>     Suspicious implicit sign extension:
        "vs->client_pf.bytes_per_pixel" with type "unsigned char" (8 bits,
        unsigned) is promoted in "vs->client_width * vs->client_height *
        vs->client_pf.bytes_per_pixel" to type "int" (32 bits, signed), then
        sign-extended to type "unsigned long" (64 bits, unsigned).  If
        "vs->client_width * vs->client_height * vs->client_pf.bytes_per_pixel"
        is greater than 0x7FFFFFFF, the upper bits of the result will all be 1.
    979             vs->client_width * vs->client_height * 
vs->client_pf.bytes_per_pixel;
    
    Change client_width / client_height to be a size_t to avoid sign
    extension and integer promotion. Then validate that dimensions are in
    range wrt the RFB protocol u16 limits.
    
    Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
    Message-id: 20180118155254.17053-1-berrange@xxxxxxxxxx
    Signed-off-by: Gerd Hoffmann <kraxel@xxxxxxxxxx>
    (cherry picked from commit 4c956bd81e2e16afd19d38d1fdeba6d9faa8a1ae)
    Signed-off-by: Michael Roth <mdroth@xxxxxxxxxxxxxxxxxx>
---
 ui/vnc.c | 9 +++++++++
 ui/vnc.h | 4 ++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index b0e12ca..06abe73 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -672,6 +672,11 @@ static void vnc_desktop_resize(VncState *vs)
         vs->client_height == pixman_image_get_height(vs->vd->server)) {
         return;
     }
+
+    assert(pixman_image_get_width(vs->vd->server) < 65536 &&
+           pixman_image_get_width(vs->vd->server) >= 0);
+    assert(pixman_image_get_height(vs->vd->server) < 65536 &&
+           pixman_image_get_height(vs->vd->server) >= 0);
     vs->client_width = pixman_image_get_width(vs->vd->server);
     vs->client_height = pixman_image_get_height(vs->vd->server);
     vnc_lock_output(vs);
@@ -2490,6 +2495,10 @@ static int protocol_client_init(VncState *vs, uint8_t 
*data, size_t len)
         return 0;
     }
 
+    assert(pixman_image_get_width(vs->vd->server) < 65536 &&
+           pixman_image_get_width(vs->vd->server) >= 0);
+    assert(pixman_image_get_height(vs->vd->server) < 65536 &&
+           pixman_image_get_height(vs->vd->server) >= 0);
     vs->client_width = pixman_image_get_width(vs->vd->server);
     vs->client_height = pixman_image_get_height(vs->vd->server);
     vnc_write_u16(vs, vs->client_width);
diff --git a/ui/vnc.h b/ui/vnc.h
index 0c33a5f..bbda054 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -278,8 +278,8 @@ struct VncState
     int last_x;
     int last_y;
     uint32_t last_bmask;
-    int client_width;
-    int client_height;
+    size_t client_width; /* limited to u16 by RFB proto */
+    size_t client_height; /* limited to u16 by RFB proto */
     VncShareMode share_mode;
 
     uint32_t vnc_encoding;
--
generated by git-patchbot for /home/xen/git/qemu-xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog