>
> On Fri, Oct 06, 2017 at 11:58:33AM +0100, Frediano Ziglio wrote:
> > Prevent possible buffer reading overflow.
> > Note that message pointer must be valid and data are checked
> > value by value so even on overflow you just get an error.
> >
> > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > ---
> > server/inputs-channel.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/server/inputs-channel.c b/server/inputs-channel.c
> > index 2de1c7c80..3d43e90ff 100644
> > --- a/server/inputs-channel.c
> > +++ b/server/inputs-channel.c
> > @@ -507,6 +507,11 @@ static bool
> > inputs_channel_handle_migrate_data(RedChannelClient *rcc,
> > SpiceMigrateDataHeader *header;
> > SpiceMigrateDataInputs *mig_data;
> >
> > + if (size < sizeof(SpiceMigrateDataHeader) +
> > sizeof(SpiceMigrateDataInputs)) {
> > + spice_warning("bad message size %u", size);
> > + return FALSE;
> > + }
> > +
>
> I would say this is a good candidate for using red_channel_warning()
> which I sent a series for ;)
> If the header is invalid, spice_error() is used, better to use
> spice_error() here too for consistency? (or the opposite).
>
>
> Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
>
> Christophe
>
Actually I think here (in similar paths) spice_error is wrong, the messages
came from the client. Basically a client can cause server crash sending
invalid messages so causing migration to fail (VM should resume in source).
Not sure that having the client failing the migration is so a nice idea.
I think migration process is usually triggered by a more powerful (an
administrator) user, the fact that a normal user can cancel the job
looks wrong to me.
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel