Libguestfs - Oct 2014 - [libhivex] Undefined behavior when accessing invalid (too small) registry hives

Hello all,

I know that one of the original design goals of libhivex was to be
resilient to corrupt, invalid, or malicious registry hives. I've
encountered some undefined behavior in libhivex when attempting to open
registry files that are too small. I'm not sure if this is a known issue
per-se or not, so I figured I'd ask here on the mailing list before I
jumped in and started adding out-of-bounds checks everywhere.

The simplest test case is when attempting to open a zero-byte registry
file, handle.c will mmap a zero-byte file and then go out of bounds while
comparing against the registry header ("regf"). I imagine even if you
pass
in a 4-byte file, the header checksum calculation will loop over 0x7F
bytes, so you'd probably encounter another error there. I guess I'm just
not sure where the ideal location(s) to place range-checking would be; is
there anything smarter than plastering checks at every read/write to the
registry file?

Or is it expected that certain sanity checks would be performed prior to
passing along any files to libhivex? What would those checks be?

Thank you,

Mahmoud Al-Qudsi
NeoSmart Technologies

On Wed, Oct 29, 2014 at 10:43:59AM -0500, Mahmoud Al-Qudsi
wrote:
> Hello all,
> 
> I know that one of the original design goals of libhivex was to be
> resilient to corrupt, invalid, or malicious registry hives. I've
> encountered some undefined behavior in libhivex when attempting to open
> registry files that are too small. I'm not sure if this is a known
issue
> per-se or not, so I figured I'd ask here on the mailing list before I
> jumped in and started adding out-of-bounds checks everywhere.
> 
> The simplest test case is when attempting to open a zero-byte registry
> file, handle.c will mmap a zero-byte file and then go out of bounds while
> comparing against the registry header ("regf"). I imagine even if
you pass
> in a 4-byte file, the header checksum calculation will loop over 0x7F
> bytes, so you'd probably encounter another error there. I guess I'm
just
> not sure where the ideal location(s) to place range-checking would be; is
> there anything smarter than plastering checks at every read/write to the
> registry file?
Oh dear, this is embarrassing.  It's a security bug (DoS) at least.

Linux seems to refuse the mmap when length == 0 and return EINVAL, but
on other OSes or if the length < 4 we would be reading outside the array.
> Or is it expected that certain sanity checks would be performed prior to
> passing along any files to libhivex? What would those checks be?
No, hivex should definitely have those checks.

I'll have a proper look at this in the morning.

Thanks,

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 Oct 29, 2014, at 3:39 PM, Richard W.M. Jones <rjones@redhat.com>
wrote:
> 
>> Or is it expected that certain sanity checks would be performed prior
to
>> passing along any files to libhivex? What would those checks be?
> 
> No, hivex should definitely have those checks.
> 
> I'll have a proper look at this in the morning.
> 
> Thanks,
> 
> Rich.
Thanks, Rich.

As far as I can tell, the only sanity checks in the initial loading of a
registry hive are the magic bits (“regf”), major_ver = 1, and the checksum
match.

When calling hivex_open with a file under 4 bytes, you run into the
out-of-bounds access when comparing against the magic bits; pass in a file 4
bytes long with “regf” correctly set, you’ll get an out-of-bounds access to
major_ver; pass in a file truncated at 0x18 (major_ver, set to 1), and you’ll
get through to the checksum routine, which will read out-of-bounds the first 128
bytes.

If you pass in a file truncated at 0x200, you’ll get past the checksum tests but
accesses (if any) to other registry header members will be out of bounds. (I
don’t think that’s the case, because that’s all unused unknown_guid stuff,
though.)

After that, offsets are checked against hdr->size; from a brief glance I’m
unsure but I think there might be an issue if the file is truncated after a page
offset. "off < h->size” will return true, but accesses to page
contents will be out-of-bounds. So I think that would need to be “off +
sizeof(ntreg_hbin_page) < h->size”?

For example, truncating a registry file at h->rootoffs  and with a
purposely-wrong hdr->offset = 0, I think you’ll get past "if (off >=
h->endpages)” and you’ll be reading the page out-of-bounds while checking
hbin magic.

I have to run, but I think there may be a few more instances of things like
this.. I know these are only reads, but I have a suspicion there’s an
out-of-bounds write somewhere along similar lines because I was getting
segfaults in some untraced code when processing bulk, untrusted registry files,
though I could be wrong.

Thanks for looking into this, I hope I haven’t led you on a wild goose chase.

Mahmoud