Libguestfs - Apr 2011 - [hivex PATCH 0/5] Fix various uninitialized data problems in hivex.

Problems were found using valgrind.  With these 5 patches, hivex can
process registry files without provoking any valgrind warnings.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
-------------- next part --------------
>From 3e941d7ef4163b8882b1296adfd837c507a81075 Mon Sep 17 00:00:00 2001
From: Richard W.M. Jones <rjones at redhat.com>
Date: Wed, 13 Apr 2011 13:54:05 +0100
Subject: [PATCH 1/5] Really fix the case where a UTF-16 string contains junk
after the string.

The previous commit b71b88f588f8660935a7d462e97b84aa2d669249 attempted
to fix this, but got the test the wrong way round so the length would
never be shorter.
---
 lib/hivex.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/hivex.c b/lib/hivex.c
index 860c85c..3f4c629 100644
--- a/lib/hivex.c
+++ b/lib/hivex.c
@@ -1335,7 +1335,7 @@ hivex_value_string (hive_h *h, hive_value_h value)
    * (Found by Hilko Bengen in a fresh Windows XP SOFTWARE hive).
    */
   size_t slen = utf16_string_len_in_bytes_max (data, len);
-  if (slen > len)
+  if (slen < len)
     len = slen;
 
   char *ret = windows_utf16_to_utf8 (data, len);
-- 
1.7.4.1

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
-------------- next part --------------
>From c22ed5a6cb58aff70bf74df5b7c1edd33d796ef4 Mon Sep 17 00:00:00 2001
From: Richard W.M. Jones <rjones at redhat.com>
Date: Wed, 13 Apr 2011 13:55:49 +0100
Subject: [PATCH 2/5] Return real length of buffer from hivex_value_value.

In real registries, often the length declared in the header does not
match the length of the block.  In this case hivex_value_value would
only allocate a value with a size which is the shorter of the two
length values, which is correct and safe.

However user code could do:

  buf = hivex_value_value (h, v, &t, &len);
  memcpy (somewhere, buf, len);

which would copy uninitialized data.

If hivex_value_value truncates a value like this, we also need to
return the shorter length to the user as well.
---
 lib/hivex.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/lib/hivex.c b/lib/hivex.c
index 3f4c629..b1f6ea6 100644
--- a/lib/hivex.c
+++ b/lib/hivex.c
@@ -1245,6 +1245,10 @@ hivex_value_value (hive_h *h, hive_value_h value,
       fprintf (stderr, "hivex_value_value: warning: declared data length
is longer than the block it is in (data 0x%zx, data len %zu, block len
%zu)\n",
                data_offset, len, blen);
     len = blen - 4;
+
+    /* Return the smaller length to the caller too. */
+    if (len_rtn)
+      *len_rtn = len;
   }
 
   char *data = h->addr + data_offset + 4;
-- 
1.7.4.1

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
-------------- next part --------------
>From 75ea457771cec140fa3376bcc299948096c07acd Mon Sep 17 00:00:00 2001
From: Richard W.M. Jones <rjones at redhat.com>
Date: Wed, 13 Apr 2011 14:03:21 +0100
Subject: [PATCH 3/5] Handle odd-length "UTF16" strings.

If the length of the buffer is not even, then this would read a byte
of uninitialized data.  Fix the length check to avoid this.
---
 lib/hivex.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/hivex.c b/lib/hivex.c
index b1f6ea6..71ea5c3 100644
--- a/lib/hivex.c
+++ b/lib/hivex.c
@@ -1384,7 +1384,7 @@ utf16_string_len_in_bytes_max (const char *str, size_t
len)
 {
   size_t ret = 0;
 
-  while (len > 0 && (str[0] || str[1])) {
+  while (len >= 2 && (str[0] || str[1])) {
     str += 2;
     ret += 2;
     len -= 2;
-- 
1.7.4.1

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
-------------- next part --------------
>From 53056244696385299fe0d298bd25053dd7c07dc0 Mon Sep 17 00:00:00 2001
From: Richard W.M. Jones <rjones at redhat.com>
Date: Wed, 13 Apr 2011 14:01:03 +0100
Subject: [PATCH 4/5] hivex_value_multiple_strings: Don't read uninitialized
data.

If hivex_value_multiple_strings was given a value which had an odd
length or if the data in the value was unterminated,
hivex_value_multiple_strings could read uninitialized data.

Potentially (although very unlikely) this could cause a
non-exploitable segfault in the calling program.
---
 lib/hivex.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/lib/hivex.c b/lib/hivex.c
index 71ea5c3..d2ab23d 100644
--- a/lib/hivex.c
+++ b/lib/hivex.c
@@ -1421,7 +1421,8 @@ hivex_value_multiple_strings (hive_h *h, hive_value_h
value)
   char *p = data;
   size_t plen;
 
-  while (p < data + len && (plen = utf16_string_len_in_bytes (p))
> 0) {
+  while (p < data + len &&
+         (plen = utf16_string_len_in_bytes_max (p, data + len - p)) > 0) {
     nr_strings++;
     char **ret2 = realloc (ret, (1 + nr_strings) * sizeof (char *));
     if (ret2 == NULL) {
-- 
1.7.4.1

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
-------------- next part --------------
>From 54ca9b34eebda11c017b854e54f8891e72f9d131 Mon Sep 17 00:00:00 2001
From: Richard W.M. Jones <rjones at redhat.com>
Date: Wed, 13 Apr 2011 14:04:16 +0100
Subject: [PATCH 5/5] Remove no longer used internal function
utf16_string_len_in_bytes.

---
 lib/hivex.c |   18 ++----------------
 1 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/lib/hivex.c b/lib/hivex.c
index d2ab23d..573c446 100644
--- a/lib/hivex.c
+++ b/lib/hivex.c
@@ -62,7 +62,6 @@
 #define HIVEX_MAX_ALLOCATION  1000000
 
 static char *windows_utf16_to_utf8 (/* const */ char *input, size_t len);
-static size_t utf16_string_len_in_bytes (const char *str);
 static size_t utf16_string_len_in_bytes_max (const char *str, size_t len);
 
 struct hive_h {
@@ -1363,23 +1362,10 @@ free_strings (char **argv)
 }
 
 /* Get the length of a UTF-16 format string.  Handle the string as
- * pairs of bytes, looking for the first \0\0 pair.
+ * pairs of bytes, looking for the first \0\0 pair.  Only read up to
+ * 'len' maximum bytes.
  */
 static size_t
-utf16_string_len_in_bytes (const char *str)
-{
-  size_t ret = 0;
-
-  while (str[0] || str[1]) {
-    str += 2;
-    ret += 2;
-  }
-
-  return ret;
-}
-
-/* As for utf16_string_len_in_bytes but only read up to a maximum length. */
-static size_t
 utf16_string_len_in_bytes_max (const char *str, size_t len)
 {
   size_t ret = 0;
-- 
1.7.4.1