[pve-devel] [PATCH v2 qemu] add fix for vnc clipboard

[pve-devel] [PATCH v2 qemu] add fix for vnc clipboard

[Fiona Ebner]

This fixes the host->guest direction with noNVC as a client (and
likely others).

Reported-by: Friedrich Weber <f.weber@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 ...-ui-vnc-clipboard-fix-inflate_buffer.patch | 34 +++++++++++++++++++
 debian/patches/series                         |  1 +
 2 files changed, 35 insertions(+)
 create mode 100644 debian/patches/extra/0010-ui-vnc-clipboard-fix-inflate_buffer.patch

diff --git a/debian/patches/extra/0010-ui-vnc-clipboard-fix-inflate_buffer.patch b/debian/patches/extra/0010-ui-vnc-clipboard-fix-inflate_buffer.patch
new file mode 100644
index 0000000..4711fa0
--- /dev/null
+++ b/debian/patches/extra/0010-ui-vnc-clipboard-fix-inflate_buffer.patch
@@ -0,0 +1,34 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Fiona Ebner <f.ebner@proxmox.com>
+Date: Wed, 22 Nov 2023 13:17:25 +0100
+Subject: [PATCH] ui/vnc-clipboard: fix inflate_buffer
+
+Commit d921fea338 ("ui/vnc-clipboard: fix infinite loop in
+inflate_buffer (CVE-2023-3255)") removed this hunk, but it is still
+required, because it can happen that stream.avail_in becomes zero
+before coming across a return value of Z_STREAM_END.
+
+This fixes the host->guest direction with noNVC.
+
+Reported-by: Friedrich Weber <f.weber@proxmox.com>
+Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
+---
+ ui/vnc-clipboard.c | 5 +++++
+ 1 file changed, 5 insertions(+)
+
+diff --git a/ui/vnc-clipboard.c b/ui/vnc-clipboard.c
+index c759be3438..124b6fbd9c 100644
+--- a/ui/vnc-clipboard.c
++++ b/ui/vnc-clipboard.c
+@@ -69,6 +69,11 @@ static uint8_t *inflate_buffer(uint8_t *in, uint32_t in_len, uint32_t *size)
+         }
+     }
+ 
++    *size = stream.total_out;
++    inflateEnd(&stream);
++
++    return out;
++
+ err_end:
+     inflateEnd(&stream);
+ err:
diff --git a/debian/patches/series b/debian/patches/series
index 992299c..9938b8e 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -7,6 +7,7 @@ extra/0006-Revert-Revert-graph-lock-Disable-locking-for-now.patch
 extra/0007-migration-states-workaround-snapshot-performance-reg.patch
 extra/0008-Revert-x86-acpi-workaround-Windows-not-handling-name.patch
 extra/0009-hw-ide-ahci-fix-legacy-software-reset.patch
+extra/0010-ui-vnc-clipboard-fix-inflate_buffer.patch
 bitmap-mirror/0001-drive-mirror-add-support-for-sync-bitmap-mode-never.patch
 bitmap-mirror/0002-drive-mirror-add-support-for-conditional-and-always-.patch
 bitmap-mirror/0003-mirror-add-check-for-bitmap-mode-without-bitmap.patch
-- 
2.39.2

Re: [pve-devel] [PATCH v2 qemu] add fix for vnc clipboard

[Friedrich Weber]

Installed a patched pve-qemu-kvm package provided by Fiona and can
confirm that the host->guest clipboard direction works now.

More specifically: Entering something in the noVNC clipboard textbox,
trying to paste in a text editor running inside a Debian Bullseye+XFCE
VM with spice-vdagent 0.20.0-2. Without the patch, the editor froze for
a few seconds and nothing was pasted. With the patch, pasting works again.

Would be great if we could get this in, as the VNC clipboard is
half-broken without it.

Tested-by: Friedrich Weber <f.weber@proxmox.com>

On 22/11/2023 13:41, Fiona Ebner wrote:
> This fixes the host->guest direction with noNVC as a client (and
> likely others).
> 
> Reported-by: Friedrich Weber <f.weber@proxmox.com>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  ...-ui-vnc-clipboard-fix-inflate_buffer.patch | 34 +++++++++++++++++++
>  debian/patches/series                         |  1 +
>  2 files changed, 35 insertions(+)
>  create mode 100644 debian/patches/extra/0010-ui-vnc-clipboard-fix-inflate_buffer.patch
> 
> diff --git a/debian/patches/extra/0010-ui-vnc-clipboard-fix-inflate_buffer.patch b/debian/patches/extra/0010-ui-vnc-clipboard-fix-inflate_buffer.patch
> new file mode 100644
> index 0000000..4711fa0
> --- /dev/null
> +++ b/debian/patches/extra/0010-ui-vnc-clipboard-fix-inflate_buffer.patch
> @@ -0,0 +1,34 @@
> +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
> +From: Fiona Ebner <f.ebner@proxmox.com>
> +Date: Wed, 22 Nov 2023 13:17:25 +0100
> +Subject: [PATCH] ui/vnc-clipboard: fix inflate_buffer
> +
> +Commit d921fea338 ("ui/vnc-clipboard: fix infinite loop in
> +inflate_buffer (CVE-2023-3255)") removed this hunk, but it is still
> +required, because it can happen that stream.avail_in becomes zero
> +before coming across a return value of Z_STREAM_END.
> +
> +This fixes the host->guest direction with noNVC.
> +
> +Reported-by: Friedrich Weber <f.weber@proxmox.com>
> +Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> +---
> + ui/vnc-clipboard.c | 5 +++++
> + 1 file changed, 5 insertions(+)
> +
> +diff --git a/ui/vnc-clipboard.c b/ui/vnc-clipboard.c
> +index c759be3438..124b6fbd9c 100644
> +--- a/ui/vnc-clipboard.c
> ++++ b/ui/vnc-clipboard.c
> +@@ -69,6 +69,11 @@ static uint8_t *inflate_buffer(uint8_t *in, uint32_t in_len, uint32_t *size)
> +         }
> +     }
> + 
> ++    *size = stream.total_out;
> ++    inflateEnd(&stream);
> ++
> ++    return out;
> ++
> + err_end:
> +     inflateEnd(&stream);
> + err:
> diff --git a/debian/patches/series b/debian/patches/series
> index 992299c..9938b8e 100644
> --- a/debian/patches/series
> +++ b/debian/patches/series
> @@ -7,6 +7,7 @@ extra/0006-Revert-Revert-graph-lock-Disable-locking-for-now.patch
>  extra/0007-migration-states-workaround-snapshot-performance-reg.patch
>  extra/0008-Revert-x86-acpi-workaround-Windows-not-handling-name.patch
>  extra/0009-hw-ide-ahci-fix-legacy-software-reset.patch
> +extra/0010-ui-vnc-clipboard-fix-inflate_buffer.patch
>  bitmap-mirror/0001-drive-mirror-add-support-for-sync-bitmap-mode-never.patch
>  bitmap-mirror/0002-drive-mirror-add-support-for-conditional-and-always-.patch
>  bitmap-mirror/0003-mirror-add-check-for-bitmap-mode-without-bitmap.patch

Re: [pve-devel] [PATCH v2 qemu] add fix for vnc clipboard

[Thomas Lamprecht]

Am 22/11/2023 um 13:45 schrieb Friedrich Weber:
> Would be great if we could get this in, as the VNC clipboard is
> half-broken without it.

not configurable via the UI though, hmm..

Re: [pve-devel] [PATCH v2 qemu] add fix for vnc clipboard

[Fiona Ebner]

Am 22.11.23 um 13:41 schrieb Fiona Ebner:
> This fixes the host->guest direction with noNVC as a client (and
> likely others).
> 
> Reported-by: Friedrich Weber <f.weber@proxmox.com>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>

Patch also sent upstream now:
https://lists.nongnu.org/archive/html/qemu-devel/2023-11/msg04845.html

[pve-devel] applied: [PATCH v2 qemu] add fix for vnc clipboard

[Thomas Lamprecht]

Am 22/11/2023 um 13:41 schrieb Fiona Ebner:
> This fixes the host->guest direction with noNVC as a client (and
> likely others).
> 
> Reported-by: Friedrich Weber <f.weber@proxmox.com>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  ...-ui-vnc-clipboard-fix-inflate_buffer.patch | 34 +++++++++++++++++++
>  debian/patches/series                         |  1 +
>  2 files changed, 35 insertions(+)
>  create mode 100644 debian/patches/extra/0010-ui-vnc-clipboard-fix-inflate_buffer.patch
> 

applied, with Friedrich's T-b, thanks!

> +diff --git a/ui/vnc-clipboard.c b/ui/vnc-clipboard.c
> +index c759be3438..124b6fbd9c 100644
> +--- a/ui/vnc-clipboard.c
> ++++ b/ui/vnc-clipboard.c
> +@@ -69,6 +69,11 @@ static uint8_t *inflate_buffer(uint8_t *in, uint32_t in_len, uint32_t *size)
> +         }
> +     }
> + 

could be a bit nicer to have a "stream_end" label here that you goto
from the Z_STREAM_END case too, making it slightly clearer how those
two cases interact (i.e., are handled exactly the same). But not
much into QEMU code (style) currently, so lets upstream handle the
review on that one, it works in any case.

> ++    *size = stream.total_out;
> ++    inflateEnd(&stream);
> ++
> ++    return out;