On Tue, 2018-07-10 at 14:35 +0100, Frediano Ziglio wrote:
> Check we have enough data before reading.
Check *that* we have...
> This could lead to read buffer overflows being undetected.
> This is not a security issue, read happens only in the client not
> causing
> any information leakage, maximum can generate a crash or some garbage
> on
> the screen.
>
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> common/canvas_base.c | 23 +++++++++++++++++++----
> 1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/common/canvas_base.c b/common/canvas_base.c
> index 05c4e1f..ae07625 100644
> --- a/common/canvas_base.c
> +++ b/common/canvas_base.c
> @@ -537,6 +537,9 @@ static pixman_image_t *canvas_get_lz4(CanvasBase
> *canvas, SpiceImage *image)
> width = image->descriptor.width;
> stride_encoded = width;
> height = image->descriptor.height;
> + if (data + 2 > data_end) {
> + return NULL;
All other error conditions that you handle below print a warning. Do
you want one here as well?
Acked-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> + }
> top_down = !!*(data++);
> spice_format = *(data++);
> switch (spice_format) {
> @@ -579,16 +582,22 @@ static pixman_image_t
> *canvas_get_lz4(CanvasBase *canvas, SpiceImage *image)
> bits = dest;
>
> do {
> + if (data + 4 > data_end) {
> + goto format_error;
> + }
> // Read next compressed block
> enc_size = read_uint32_be(data);
> data += 4;
> + /* check overflow. This check is a bit different to avoid
> + * possible overflows. From previous check data_end - data
> cannot overflow.
> + * Computing data + enc_size on 32 bit could cause
> overflows. */
> + if (enc_size < 0 || data_end - data < (unsigned int)
> enc_size) {
> + goto format_error;
> + }
> dec_size = LZ4_decompress_safe_continue(stream, (const char
> *) data,
> (char *) dest,
> enc_size, available);
> if (dec_size <= 0) {
> - spice_warning("Error decoding LZ4 block\n");
> - pixman_image_unref(surface);
> - surface = NULL;
> - break;
> + goto format_error;
> }
> dest += dec_size;
> available -= dec_size;
> @@ -599,6 +608,12 @@ static pixman_image_t *canvas_get_lz4(CanvasBase
> *canvas, SpiceImage *image)
>
> LZ4_freeStreamDecode(stream);
> return surface;
> +
> +format_error:
> + spice_warning("Error decoding LZ4 block\n");
> + LZ4_freeStreamDecode(stream);
> + pixman_image_unref(surface);
> + return NULL;
> }
> #endif
>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel