Libguestfs - Dec 2016 - Re: increasing HIVEX_MAX_SUBKEYS and HIVEX_MAX_VALUES

Hello,

We've been seeing an increasing number of software hives containing subkey
and value counts that exceed the limits defined in hivex-internal.h
(https://github.com/libguestfs/hivex/blob/1.3.13/lib/hivex-internal.h#L325-L327).
Since hivex abruptly aborts when it comes across a key with more subkeys or
values than the defined limits, it's breaking some functionality in our
cloud.

The purpose of this email is to open a discussion about these arbitrary limits
and propose both a short-term hotfix and a long-term solution.

This issue has only been observed in our cloud on servers running Windows 2008R2
and newer (particularly systems with Exchange installed), but very few of the
registry keys causing the problem are specific to server editions of Windows.
It's likely that this could also happen on workstation variants and possible
that we've only seen it on server variants, since that's what the
majority of our cloud is comprised of.

Our code is particularly interested in 'Microsoft\Windows
NT\CurrentVersion' within the software hive. Among the set of hives that
hivex is unable to process, the largest number of subkeys found within
'Microsoft\Windows NT\CurrentVersion' is 45962, and the largest number
of values is 37717.

If I check the entire software hive, I find a maximum of 2393689 subkeys and
74324 values.

I feel a conservative approach would be to increase the limits to approximately
1.5 times the largest observed counts:
  • If we decide to go with the values observed from just the
'Microsoft\Windows NT\CurrentVersion' key in our cloud, then
HIVEX_MAX_SUBKEYS would become 70000 and HIVEX_MAX_VALUES would become 55000.
These are the initial values I was going to base my patch on, since (selfishly)
that's all my code cares about, but I figured it makes more sense to account
for all regions of the hive.
  • If we decide to go with the values from the whole software hive, then
HIVEX_MAX_SUBKEYS would become 3600000 and HIVEX_MAX_VALUES would become 110000.
These are the values I used in the attached patch. The only thing that makes me
hesitant is that they're both so significantly larger than the current
limits.

What about removing the limits entirely? The registry format allows up to 2^32
subkeys and values. On IRC, rwmjones said, "the limits are there to stop
malicious hives from using too much memory". I hadn't heard of
malicious hives before. Have there been exploits that leveraged this, or is it
just being cautious about a potential threat?

rwmjones suggested that the ideal solution would be adding the ability to set
the limits at runtime or ignore them entirely. I agree that that's a better
solution in the long run, but involves a much more significant reworking of the
code than simply increasing some defined values.

Since we need a fix for our cloud in the short term and it might help other
people experiencing the same problem, I propose two patches:
1. bump the limits to 1.5 times the highest observed counts; I've attached a
patch that bumps HIVEX_MAX_SUBKEYS to 3.6m and HIVEX_MAX_VALUES to 110k
2. in a future patch, allow setting the limits or disabling them entirely at
runtime

-- 
Matt Coleman
Senior Software Engineer
Datto, Inc.
www.datto.com

On Fri, Dec 02, 2016 at 06:59:32PM -0500, Matt Coleman
wrote:
> I feel a conservative approach would be to increase the limits to
> approximately 1.5 times the largest observed counts:
>
> • If we decide to go with the values observed from just the
> 'Microsoft\Windows NT\CurrentVersion' key in our cloud, then
> HIVEX_MAX_SUBKEYS would become 70000 and HIVEX_MAX_VALUES would
> become 55000. These are the initial values I was going to base
> my patch on, since (selfishly) that's all my code cares about,
> but I figured it makes more sense to account for all regions of
> the hive.
This is reasonable and safe.
> • If we decide to go with the values from the whole software hive,
> then HIVEX_MAX_SUBKEYS would become 3600000 and HIVEX_MAX_VALUES
> would become 110000. These are the values I used in the attached
> patch. The only thing that makes me hesitant is that they're
> both so significantly larger than the current limits.
These are getting rather large.  As discussed on IRC if we're going
for this kind of approach I would rather that the limits were made
completely configurable through the API, so that callers who know what
they're doing can increase them (or even remove them), while those
that don't won't get bitten by a malicious hive that causes a denial
of service.
> What about removing the limits entirely? The registry format allows
> up to 2^32 subkeys and values. On IRC, rwmjones said, "the limits
> are there to stop malicious hives from using too much memory". I
> hadn't heard of malicious hives before. Have there been exploits
> that leveraged this, or is it just being cautious about a potential
> threat?
We are very much concerned about malicious hives because tools built
on top of hivex (such as libguestfs) are frequently run against
untrusted guests, eg in cloud situations like OpenStack.

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/

>  /* These limits are in place to stop really stupid stuff and/or exploits.
*/
> -#define HIVEX_MAX_SUBKEYS       25000
> -#define HIVEX_MAX_VALUES        10000
> +#define HIVEX_MAX_SUBKEYS     3600000
> +#define HIVEX_MAX_VALUES       110000
>  #define HIVEX_MAX_VALUE_LEN   8000000
>  #define HIVEX_MAX_ALLOCATION  1000000
Do you have the alternate patch that just increases the limits to 1.5x
what you need?

We're building a list of (up to) HIVEX_MAX_SUBKEYS * 4 bytes (not
counting intermediate blocks) in the _get_children function.  While
it's not a massive amount of memory in a modern system, I think if
we're going to increase them substantially we should make these limits
configurable which makes everyone happy.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

> Do you have the alternate patch that just increases the limits to 1.5x what
you need?
I've attached an updated patch that increases the limits based on the counts
observed in the Microsoft\Windows NT\CurrentVersion subkey of the software hive.
> We're building a list of (up to) HIVEX_MAX_SUBKEYS * 4 bytes (not
counting intermediate blocks) in the _get_children function.  While it's not
a massive amount of memory in a modern system, I think if we're going to
increase them substantially we should make these limits configurable which makes
everyone happy.
Sounds good. I'll start looking at making it configurable.

Matt