Re: [PATCH 3/3] ui/vnc-enc-tight: Avoid dynamic stack allocation

[Francisco Iglesias]

On [2023 Aug 18] Fri 16:10:57, Peter Maydell wrote:
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Use autofree heap allocation instead of variable-length
> array on the stack.
> 
> The codebase has very few VLAs, and if we can get rid of them all we
> can make the compiler error on new additions.  This is a defensive
> measure against security bugs where an on-stack dynamic allocation
> isn't correctly size-checked (e.g.  CVE-2021-3527).
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> [PMM: expanded commit message]
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

> ---
>  ui/vnc-enc-tight.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
> index ee853dcfcb8..41f559eb837 100644
> --- a/ui/vnc-enc-tight.c
> +++ b/ui/vnc-enc-tight.c
> @@ -1097,13 +1097,13 @@ static int send_palette_rect(VncState *vs, int x, int 
> y,
>      switch (vs->client_pf.bytes_per_pixel) {
>      case 4:
>      {
> -        size_t old_offset, offset;
> -        uint32_t header[palette_size(palette)];
> +        size_t old_offset, offset, palette_sz = palette_size(palette);
> +        g_autofree uint32_t *header = g_new(uint32_t, palette_sz);
>          struct palette_cb_priv priv = { vs, (uint8_t *)header };
>  
>          old_offset = vs->output.offset;
>          palette_iter(palette, write_palette, &priv);
> -        vnc_write(vs, header, sizeof(header));
> +        vnc_write(vs, header, palette_sz * sizeof(uint32_t));
>  
>          if (vs->tight->pixel24) {
>              tight_pack24(vs, vs->output.buffer + old_offset, colors, 
> &offset);
> @@ -1115,11 +1115,12 @@ static int send_palette_rect(VncState *vs, int x, int 
> y,
>      }
>      case 2:
>      {
> -        uint16_t header[palette_size(palette)];
> +        size_t palette_sz = palette_size(palette);
> +        g_autofree uint16_t *header = g_new(uint16_t, palette_sz);
>          struct palette_cb_priv priv = { vs, (uint8_t *)header };
>  
>          palette_iter(palette, write_palette, &priv);
> -        vnc_write(vs, header, sizeof(header));
> +        vnc_write(vs, header, palette_sz * sizeof(uint16_t));
>          tight_encode_indexed_rect16(vs->tight->tight.buffer, w * h, palette);
>          break;
>      }
> -- 
> 2.34.1
> 
>