[libvirt] [PATCHv2] uml: sanity check external data before using it

[Chris Lalancette]

On 03/03/2010 11:52 AM, Eric Blake wrote:
> Otherwise, a malicious packet could cause a DoS via spurious
> out-of-memory failure.
> 
> * src/uml/uml_driver.c (umlMonitorCommand): Validate that incoming
> data is reliable before using it to allocate/dereference memory.
> Don't report bogus errno on short read.
> Reported by Jim Meyering.
> ---
>  src/uml/uml_driver.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
> index eec239f..4a5db9d 100644
> --- a/src/uml/uml_driver.c
> +++ b/src/uml/uml_driver.c
> @@ -740,15 +740,15 @@ static int umlMonitorCommand(virConnectPtr conn,
>          if (nbytes < 0) {
>              if (errno == EAGAIN || errno == EINTR)
>                  continue;
> -            virReportSystemError(errno,
> -                                 _("cannot read reply %s"),
> -                                 cmd);
> +            virReportSystemError(errno, _("cannot read reply %s"), cmd);
>              goto error;
>          }
>          if (nbytes < sizeof res) {
> -            virReportSystemError(errno,
> -                                 _("incomplete reply %s"),
> -                                 cmd);
> +            virReportSystemError(0, _("incomplete reply %s"), cmd);
> +            goto error;
> +        }
> +        if (sizeof res.data < res.length) {
> +            virReportSystemError(0, _("invalid length in reply %s"), cmd);
>              goto error;
>          }

Let me preface this by saying that this is the first time I've ever looked at
UML.  With that being said, I'm not sure this above check is correct.
sizeof(res.data) is always a constant 512, but res.length may or may not vary
based on the message.  That is, it looks to me like it's perfectly valid
for res.length to be less than res.data.  In point of fact the code in
uml_mconsole.c (which is where this was ported from) ignores res.length altogether
and just uses strlen(res.data) to realloc and copy.  I'd be wary of pushing
this code without testing it out under UML.

-- 
Chris Lalancette