[Spice-devel] [PATCH spice-server 3/3] Dispatcher: validate received message types

[Frediano Ziglio]

> 
> Although dispatcher_send_message() does not allow you to send a message
> type that is invalid for a dispatcher, it still makes sense to be
> defensive in the receiver. Validate that the message type that was read
> from the socket is in fact a valid message type for this dispatcher.
> ---
>  server/dispatcher.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/server/dispatcher.c b/server/dispatcher.c
> index 4e03ea046..965bae8e8 100644
> --- a/server/dispatcher.c
> +++ b/server/dispatcher.c
> @@ -285,6 +285,10 @@ static int dispatcher_handle_single_read(Dispatcher
> *dispatcher)
>          /* no messsage */
>          return 0;
>      }
> +    if (type >= dispatcher->priv->max_message_type) {
> +        spice_printerr("Invalid message type for this dispatcher: %u",
> type);
> +        return 0;
> +    }
>      msg = &dispatcher->priv->messages[type];
>      if (read_safe(dispatcher->priv->recv_fd, payload, msg->size, 1) == -1) {
>          spice_printerr("error reading from dispatcher: %d", errno);

This patch looks like not really defensive. The events where this can occurs
in my mind are:
- physical damage (memory/cpu);
- memory corruption;
- file descriptor messing.
All quite critical. You print an error and return leaving the state
inconsistent potentially opening the door at any breach.
I would use a spice_error, process should die.

On the paranoia level I would use an UNLIKELY macro to hint the compiler.

Frediano