diff options
author | Daniel P. Berrangé <berrange@redhat.com> | 2018-02-05 11:49:37 +0000 |
---|---|---|
committer | Gerd Hoffmann <kraxel@redhat.com> | 2018-02-16 12:33:02 +0100 |
commit | cf0706581bc0c24ab2e9a81ff0fc3efa9482c812 (patch) | |
tree | e3cb993a75f3bc55d57c64ab8a884418edc547f6 | |
parent | 52c7c9d076dc64a6d3f1938b5a4994f84744c7fa (diff) | |
download | qemu-cf0706581bc0c24ab2e9a81ff0fc3efa9482c812.zip qemu-cf0706581bc0c24ab2e9a81ff0fc3efa9482c812.tar.gz qemu-cf0706581bc0c24ab2e9a81ff0fc3efa9482c812.tar.bz2 |
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: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20180205114938.15784-4-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
-rw-r--r-- | ui/vnc.c | 22 |
1 files changed, 13 insertions, 9 deletions
@@ -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)); |