mrc0mmand / rpms / hivex

Forked from rpms/hivex 3 years ago
Clone
Blob Blame History Raw
From 690e51511c532194fcff6450fe4e272a58920ba4 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Thu, 15 Apr 2021 15:50:13 +0100
Subject: [PATCH 15/15] lib/handle.c: Bounds check for block exceeding page
 length (CVE-2021-3504)

Hives are encoded as fixed-sized pages containing smaller variable-
length blocks:

  +-------------------+-------------------+-------------------+--
  | header            |[ blk ][blk][ blk ]|[blk][blk][blk]    |
  +-------------------+-------------------+-------------------+--

Blocks should not straddle a page boundary.  However because blocks
contain a 32 bit length field it is possible to construct an invalid
hive where the last block in a page overlaps either the next page or
the end of the file:

  +-------------------+-------------------+
  | header            |[ blk ][blk][ blk ..... ]
  +-------------------+-------------------+

Hivex lacked a bounds check and would process the registry.  Because
the rest of the code assumes this situation can never happen it was
possible to have a block containing some field (eg. a registry key
name) which would extend beyond the end of the file.  Hivex mmaps or
mallocs the file, causing hivex to read memory beyond the end of the
mapped region, resulting in reading other memory structures or a
crash.  (Writing beyond the end of the mapped region seems to be
impossible because we always allocate a new page before writing.)

This commit adds a check which rejects the malformed registry on
hivex_open.

Credit: Jeremy Galindo, Sr Security Engineer, Datto.com
Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
Fixes: CVE-2021-3504
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1949687
---
 lib/handle.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/lib/handle.c b/lib/handle.c
index 0d2b24b..b02808e 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -315,8 +315,8 @@ hivex_open (const char *filename, int flags)
       if (seg_len <= 4 || (seg_len & 3) != 0) {
         if (is_root || !h->unsafe) {
           SET_ERRNO (ENOTSUP,
-                     "%s, the block at 0x%zx has invalid size %" PRIi32
-                     ", bad registry",
+                     "%s, the block at 0x%zx size %" PRIi32
+                     " <= 4 or not a multiple of 4, bad registry",
                      filename, blkoff, le32toh (block->seg_len));
           goto error;
         } else {
@@ -327,6 +327,14 @@ hivex_open (const char *filename, int flags)
         }
       }
 
+      if (blkoff + seg_len > off + page_size) {
+        SET_ERRNO (ENOTSUP,
+                   "%s, the block at 0x%zx size %" PRIi32
+                   " extends beyond the current page, bad registry",
+                   filename, blkoff, le32toh (block->seg_len));
+        goto error;
+      }
+
       if (h->msglvl >= 2) {
         unsigned char *id = (unsigned char *) block->id;
         int id0 = id[0], id1 = id[1];
-- 
1.8.3.1