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

[Richard W.M. Jones]

On Thu, Feb 16, 2017 at 11:04:12AM -0500, Dawid Zamirski wrote:
> On Thu, 2017-02-16 at 08:43 +0000, Richard W.M. Jones wrote:
> > On Wed, Feb 15, 2017 at 10:59:33PM +0000, Richard W.M. Jones wrote:
> > > 
> > > 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.
> > 
> > Also if we're going to start using heuristics to deal with broken
> > hives, we should prevent writing when this happens.  So check the
> > write flag and give an error in that case (or have another flag to
> > indicate that the caller wants heuristics).
> > 
> > Rich.
> > 
> 
> In this case, I'd opt for a new flag because in our use case we still
> might want to modify such hives - we do something similar to v2v on
> backup images.

Yup, agreed.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/