Libguestfs - Feb 2017 - Re: [PATCH v2 1/2] lib: change how hbin sections are read.

On Wed, Feb 15, 2017 at 10:29:41PM +0000, Richard W.M. Jones
wrote:
> Yes, or even how about this (not tried it):
> 
> while (off <= h->endpages - 0x1000) {
>   ...
> }
In fact this doesn't work either :-(

I'll have another look at this tomorrow morning.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

On Wed, 2017-02-15 at 22:35 +0000, Richard W.M. Jones
wrote:
> On Wed, Feb 15, 2017 at 10:29:41PM +0000, Richard W.M. Jones wrote:
> > Yes, or even how about this (not tried it):
> > 
> > while (off <= h->endpages - 0x1000) {
> >   ...
> > }
> 
> In fact this doesn't work either :-(
> 
> I'll have another look at this tomorrow morning.
> 
> Rich.
> 
Yep, GCC7 complains about that off could overflow over SIZE_MAX when
incremented with 0x1000 and could cause infinite loop, i.e

size_t off = SIZE_MAX - 50;
size_t endpages = SIZE_MAX;
off += 100; // off is now 50

if (off < endpages)
   prinf("off is still smaller!");

To prevent this the while loop could be written as:

while (off + 0x1000 < off && off < h->endpages) {
  off += 0x1000; 

OK, I ended up turning the warning off.  It appears from the
info file that the warning is about GCC not being able to make
an optimization, not a bug in the code.

However I do have a more substantial problem with the patch.
By checking the offset against h->endpages, we're using an untrusted
field supplied to us by the hive, which means that a crafted hive
could cause us to walk through memory past the end of the file --
a security issue.

So I think the test should be using h->size with the additional
check for off >= h->endpages, as in the existing outer loop.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top