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

[Chris Lalancette]

On 03/09/2010 04:31 PM, Eric Blake wrote:
> On 03/09/2010 12:17 PM, Chris Lalancette wrote:
>>>          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.
> 
> I'm with you in the fresh eyes category on this one :)
> 
>>  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.
> 
> That is a true statement.  But this check is whether res.length is
> _larger_ than 512, in which case the packet is bogus.

Gah, I looked at it backwards.  You are right of course.  Still, we
might want to verify that res.length is valid.

> 
>>  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.
> 
> Possibly a valid approach, but the version in this patch was the one
> recommended by Jim.  For that matter, is strlen(res.data) safe, or can
> it run past the end of res.length bytes if the user (maliciously) failed
> to pass in a NUL terminator?

This is a good point too.  I don't know the answer.

> 
>>  I'd be wary of pushing
>> this code without testing it out under UML.
> 
> All right then - any advice from someone who HAS used UML on how to test
> this beyond mere code inspection?  Related question: does libvirt-tck
> exercise UML?

I know danpb originally wrote the code, and does use it.  Whether libvirt-tck
exercises it is unknown to me.

-- 
Chris Lalancette