Commit cf070658 authored by Daniel P. Berrangé's avatar Daniel P. Berrangé Committed by Gerd Hoffmann
Browse files

ui: check VNC audio frequency limit at time of reading from client 



The 'vs->as.freq' value is a signed integer, which is read from an
unsigned 32-bit int field on the wire. There is thus a risk of overflow
on 32-bit platforms. Move the frequency limit checking to be done at
time of read before casting to a signed integer.

Reported-by: default avatarLaszlo Ersek <lersek@redhat.com>
Signed-off-by: default avatarDaniel P. Berrangé <berrange@redhat.com>
Reviewed-by: default avatarLaszlo Ersek <lersek@redhat.com>
Reviewed-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20180205114938.15784-4-berrange@redhat.com
Signed-off-by: default avatarGerd Hoffmann <kraxel@redhat.com>
parent 52c7c9d0

ui/vnc.c

+13 −9
Original line number Diff line number Diff line
@@ -982,14 +982,7 @@ static void vnc_update_throttle_offset(VncState *vs) 
        vs->client_width * vs->client_height * vs->client_pf.bytes_per_pixel;



    if (vs->audio_cap) {

        int freq = vs->as.freq;

        /* We don't limit freq when reading settings from client, so

         * it could be upto MAX_INT in size. 48khz is a sensible

         * upper bound for trustworthy clients */

        int bps;

        if (freq > 48000) {

            freq = 48000;

        }

        switch (vs->as.fmt) {

        default:

        case  AUD_FMT_U8:
@@ -1005,7 +998,7 @@ static void vnc_update_throttle_offset(VncState *vs) 
            bps = 4;

            break;

        }

        offset += freq * bps * vs->as.nchannels;

        offset += vs->as.freq * bps * vs->as.nchannels;

    }



    /* Put a floor of 1MB on offset, so that if we have a large pending
@@ -2292,6 +2285,7 @@ static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len) 
{

    int i;

    uint16_t limit;

    uint32_t freq;

    VncDisplay *vd = vs->vd;



    if (data[0] > 3) {
@@ -2411,7 +2405,17 @@ static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len) 
                    vnc_client_error(vs);

                    break;

                }

                vs->as.freq = read_u32(data, 6);

                freq = read_u32(data, 6);

                /* No official limit for protocol, but 48khz is a sensible

                 * upper bound for trustworthy clients, and this limit

                 * protects calculations involving 'vs->as.freq' later.

                 */

                if (freq > 48000) {

                    VNC_DEBUG("Invalid audio frequency %u > 48000", freq);

                    vnc_client_error(vs);

                    break;

                }

                vs->as.freq = freq;

                break;

            default:

                VNC_DEBUG("Invalid audio message %d\n", read_u8(data, 4));