| Message ID | 1479749115-21932-1-git-send-email-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 11/21/2016 11:25 AM, Thomas Huth wrote: > If the buffer is not big enough, snprintf() does not return the number > of bytes that have been written to the buffer, but the number of bytes > that would be needed for writing the whole string. By using this value > for the following vnc_write() calls, we send some junk at the end of > the name in case the qemu_name is longer than 1017 bytes, which could > confuse the VNC clients. Fix this by adding an additional size check > here. > > Buglink: https://bugs.launchpad.net/qemu/+bug/1637447 > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > ui/vnc.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) Reviewed-by: Eric Blake <eblake@redhat.com> Worth having in 2.8, I think.
On Mo, 2016-11-21 at 18:25 +0100, Thomas Huth wrote: > If the buffer is not big enough, snprintf() does not return the number > of bytes that have been written to the buffer, but the number of bytes > that would be needed for writing the whole string. By using this value > for the following vnc_write() calls, we send some junk at the end of > the name in case the qemu_name is longer than 1017 bytes, which could > confuse the VNC clients. Fix this by adding an additional size check > here. Use g_strdup_printf instead? cheers, Gerd
On 21.11.2016 18:51, Gerd Hoffmann wrote: > On Mo, 2016-11-21 at 18:25 +0100, Thomas Huth wrote: >> If the buffer is not big enough, snprintf() does not return the number >> of bytes that have been written to the buffer, but the number of bytes >> that would be needed for writing the whole string. By using this value >> for the following vnc_write() calls, we send some junk at the end of >> the name in case the qemu_name is longer than 1017 bytes, which could >> confuse the VNC clients. Fix this by adding an additional size check >> here. > > Use g_strdup_printf instead? Not sure ... If I'd use g_strdup_printf instead here, we might end up with a string that is bigger than 1024 bytes here. Maybe there is/was a reason that the string is limited to 1024 bytes here? Are there clients that can not deal with more that 1024 bytes? So unless you feel very confident that it does not matter, using the size check instead of g_strdup_printf sounds like the less invasive solution to me. Thomas
On Mo, 2016-11-21 at 18:25 +0100, Thomas Huth wrote: > If the buffer is not big enough, snprintf() does not return the number > of bytes that have been written to the buffer, but the number of bytes > that would be needed for writing the whole string. By using this value > for the following vnc_write() calls, we send some junk at the end of > the name in case the qemu_name is longer than 1017 bytes, which could > confuse the VNC clients. Fix this by adding an additional size check > here. Added to ui queue. thanks, Gerd
diff --git a/ui/vnc.c b/ui/vnc.c index 2c28a59..29aa9c4 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -2459,10 +2459,14 @@ static int protocol_client_init(VncState *vs, uint8_t *data, size_t len) pixel_format_message(vs); - if (qemu_name) + if (qemu_name) { size = snprintf(buf, sizeof(buf), "QEMU (%s)", qemu_name); - else + if (size > sizeof(buf)) { + size = sizeof(buf); + } + } else { size = snprintf(buf, sizeof(buf), "QEMU"); + } vnc_write_u32(vs, size); vnc_write(vs, buf, size);
If the buffer is not big enough, snprintf() does not return the number of bytes that have been written to the buffer, but the number of bytes that would be needed for writing the whole string. By using this value for the following vnc_write() calls, we send some junk at the end of the name in case the qemu_name is longer than 1017 bytes, which could confuse the VNC clients. Fix this by adding an additional size check here. Buglink: https://bugs.launchpad.net/qemu/+bug/1637447 Signed-off-by: Thomas Huth <thuth@redhat.com> --- ui/vnc.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)