>
> On Tue, 2018-07-10 at 14:35 +0100, Frediano Ziglio wrote:
> > Check we have enough data before reading.
>
> Check *that* we have...
>
updated, thanks
> > 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>
>
does
g_warning("missing header in LZ4 data");
look fine?
Frediano
>
> > + }
> > 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