Accepting request 986262 from Virtualization

Fix for bsc#1201064 VUL-0: CVE-2022-2211: Libguestfs: Buffer overflow in get_keys leads to DOS

OBS-URL: https://build.opensuse.org/request/show/986262
OBS-URL: https://build.opensuse.org/package/show/openSUSE:Factory/libguestfs?expand=0&rev=82

CVE-2022-2211-docs-guestfs-security-document.patch

@@ -0,0 +1,55 @@
+Subject: docs/guestfs-security: document CVE-2022-2211
+From: Laszlo Ersek lersek@redhat.com Tue Jun 28 13:54:16 2022 +0200
+Date: Wed Jun 29 15:29:37 2022 +0200:
+Git: 99844660b48ed809e37378262c65d63df6ce4a53
+
+Short log for the common submodule, commit range
+f8de5508fe75..35467027f657:
+
+Laszlo Ersek (2):
+      mlcustomize: factor out pkg install/update/uninstall from guestfs-tools
+      options: fix buffer overflow in get_keys() [CVE-2022-2211]
+
+Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1809453
+Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2100862
+Signed-off-by: Laszlo Ersek <lersek@redhat.com>
+Message-Id: <20220628115418.5376-2-lersek@redhat.com>
+Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
+
+--- a/docs/guestfs-security.pod
++++ b/docs/guestfs-security.pod
+@@ -406,6 +406,34 @@ The libvirt backend is not affected.
+ The solution is to update qemu to a version containing the fix (see
+ L<https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg01012.html>).
+ 
++=head2 CVE-2022-2211
++
++L<https://bugzilla.redhat.com/CVE-2022-2211>
++
++The C<get_keys> function in F<libguestfs-common/options/keys.c> collects
++those I<--key> options from the command line into a new array that match
++a particular block device that's being decrypted for inspection. The
++function intends to size the result array such that potentially all
++I<--key> options, plus a terminating C<NULL> element, fit into it. The
++code mistakenly uses the C<MIN> macro instead of C<MAX>, and therefore
++only one element is allocated before the C<NULL> terminator.
++
++Passing precisely two I<--key ID:...> options on the command line for
++the encrypted block device C<ID> causes C<get_keys> to overwrite the
++terminating C<NULL>, leading to an out-of-bounds read in
++C<decrypt_mountables>, file F<libguestfs-common/options/decrypt.c>.
++
++Passing more than two I<--key ID:...> options on the command line for
++the encrypted block device C<ID> causes C<get_keys> itself to perform
++out-of-bounds writes. The most common symptom is a crash with C<SIGSEGV>
++later on.
++
++This issue affects -- broadly speaking -- all libguestfs-based utilities
++that accept I<--key>, namely: C<guestfish>, C<guestmount>, C<virt-cat>,
++C<virt-customize>, C<virt-diff>, C<virt-edit>, C<virt-get-kernel>,
++C<virt-inspector>, C<virt-log>, C<virt-ls>, C<virt-sparsify>,
++C<virt-sysprep>, C<virt-tail>, C<virt-v2v>.
++
+ =head1 SEE ALSO
+ 
+ L<guestfs(3)>,

CVE-2022-2211-options-fix-buffer-overflow-in-get_keys.patch

@@ -0,0 +1,116 @@
+Subject: options: fix buffer overflow in get_keys() [CVE-2022-2211]
+From: Laszlo Ersek lersek@redhat.com Tue Jun 28 13:49:04 2022 +0200
+Date: Wed Jun 29 15:17:17 2022 +0200:
+Git: 35467027f657de76aca34b48a6f23e9608b23a57
+
+When calculating the greatest possible number of matching keys in
+get_keys(), the current expression
+
+  MIN (1, ks->nr_keys)
+
+is wrong -- it will return at most 1.
+
+If all "nr_keys" keys match however, then we require "nr_keys" non-NULL
+entries in the result array; in other words, we need
+
+  MAX (1, ks->nr_keys)
+
+(The comment just above the expression is correct; the code is wrong.)
+
+This buffer overflow is easiest to trigger in those guestfs tools that
+parse the "--key" option in C; that is, with "OPTION_key". For example,
+the command
+
+$ virt-cat $(seq -f '--key /dev/sda2:key:%g' 200) -d DOMAIN /no-such-file
+
+which passes 200 (different) passphrases for the LUKS-encrypted block
+device "/dev/sda2", crashes with a SIGSEGV.
+
+A slightly better reproducer from Rich Jones is the following, since it
+doesn't require an encrypted guest disk image:
+
+$ echo TEST | guestfish --keys-from-stdin -N part luks-format /dev/sda1 0
+$ virt-cat $(seq -f '--key /dev/sda1:key:%g' 200) -a test1.img /no-such-file
+Segmentation fault (core dumped)
+$ rm test1.img
+
+(
+
+  The buffer overflow is possible to trigger in OCaml-language tools as
+  well; that is, those that call "create_standard_options" with
+  ~key_opts:true.
+
+  Triggering the problem that way is less trivial. The reason is that when
+  the OCaml tools parse the "--key" options, they de-duplicate the options
+  first, based on the device identifier.
+
+  Thus, in theory, this de-duplication masks the issue, as (one would
+  think) only one "--key" option could belong to a single device, and
+  therefore the buffer overflow would not be triggered in practice.
+
+  This is not the case however: the de-duplication does not collapse keys
+  that are provided for the same device, but use different identifier
+  types (such as pathname of device node versus LUKS UUID) -- in that
+  situation, two entries in the keystore will match the device, and the
+  terminating NULL entry will not be present once get_keys() returns. In
+  this scenario, we don't have an out-of-bounds write, but an
+  out-of-bounds read, in decrypt_mountables() [options/decrypt.c].
+
+  There is *yet another* bug in get_keys() though that undoes the above
+  "masking". The "uuid" parameter of get_keys() may be NULL (for example
+  when the device to decrypt uses BitLocker and not LUKS). When this
+  happens, get_keys() adds all keys in the keystore to the result array.
+  Therefore, the out-of-bounds write is easy to trigger with
+  OCaml-language tools as well, as long as we attempt to decrypt a
+  BitLocker (not LUKS) device, and we pass the "--key" options with
+  different device identifiers.
+
+  Subsequent patches in this series fix all of the above; this patch fixes
+  the security bug.
+
+)
+
+Rather than replacing MIN with MAX, open-code the comparison, as we first
+set "len" to 1 anyway.
+
+While at it, rework the NULL-termination such that the (len+1) addition
+not go unchecked.
+
+Fixes: c10c8baedb88e7c2988a01b70fc5f81fa8e4885c
+Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1809453
+Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2100862
+Signed-off-by: Laszlo Ersek <lersek@redhat.com>
+Message-Id: <20220628114915.5030-2-lersek@redhat.com>
+Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
+
+--- a/common/options/keys.c
++++ b/common/options/keys.c
+@@ -128,17 +128,23 @@ read_first_line_from_file (const char *f
+ char **
+ get_keys (struct key_store *ks, const char *device, const char *uuid)
+ {
+-  size_t i, j, len;
++  size_t i, j, nmemb;
+   char **r;
+   char *s;
+ 
+   /* We know the returned list must have at least one element and not
+    * more than ks->nr_keys.
+    */
+-  len = 1;
+-  if (ks)
+-    len = MIN (1, ks->nr_keys);
+-  r = calloc (len+1, sizeof (char *));
++  nmemb = 1;
++  if (ks && ks->nr_keys > nmemb)
++    nmemb = ks->nr_keys;
++
++  /* make room for the terminating NULL */
++  if (nmemb == (size_t)-1)
++    error (EXIT_FAILURE, 0, _("size_t overflow"));
++  nmemb++;
++
++  r = calloc (nmemb, sizeof (char *));
+   if (r == NULL)
+     error (EXIT_FAILURE, errno, "calloc");
+ 

libguestfs.changes

@@ -1,3 +1,11 @@
+-------------------------------------------------------------------
+Wed Jun 29 09:51:03 MDT 2022 - carnold@suse.com
+
+- bsc#1201064 - Libguestfs: Buffer overflow in get_keys leads
+  to DOS - CVE-2022-2211
+  CVE-2022-2211-options-fix-buffer-overflow-in-get_keys.patch
+  CVE-2022-2211-docs-guestfs-security-document.patch
+
 -------------------------------------------------------------------
 Fri Jun  3 15:28:15 MDT 2022 - carnold@suse.com
 

libguestfs.spec

@@ -32,6 +32,8 @@ Source100:      mount-rootfs-and-chroot.sh
 Source101:      README
 
 # Patches
+Patch1:         CVE-2022-2211-options-fix-buffer-overflow-in-get_keys.patch
+Patch2:         CVE-2022-2211-docs-guestfs-security-document.patch
 
 BuildRequires:  bison
 BuildRequires:  file-devel