Blame SOURCES/0175-fix-memory-corruption-in-pubkey-filter-over-network.patch

ecb9bb
From 27d2434ea9aed09b6cefdcc8600e191fa6d1c2fb Mon Sep 17 00:00:00 2001
ecb9bb
From: Andrei Borzenkov <arvidjaar@gmail.com>
ecb9bb
Date: Fri, 5 Dec 2014 21:17:08 +0300
ecb9bb
Subject: [PATCH 18/23] fix memory corruption in pubkey filter over network
ecb9bb
ecb9bb
grub_pubkey_open closed original file after it was read; it set
ecb9bb
io->device to NULL to prevent grub_file_close from trying to close device.
ecb9bb
But network device itself is stacked (net -> bufio); and bufio preserved
ecb9bb
original netfs file which hold reference to device. grub_file_close(io)
ecb9bb
called grub_bufio_close which called grub_file_close for original file.
ecb9bb
grub_file_close(netfs-file) now also called grub_device_close which
ecb9bb
freed file->device->net. So file structure returned by grub_pubkey_open
ecb9bb
now had device->net pointed to freed memory. When later file was closed,
ecb9bb
it was attempted to be freed again.
ecb9bb
ecb9bb
Change grub_pubkey_open to behave like other filters - preserve original
ecb9bb
parent file and pass grub_file_close down to parent. In this way only the
ecb9bb
original file will close device. We really need to move this logic into
ecb9bb
core instead.
ecb9bb
ecb9bb
Also plug memory leaks in error paths on the way.
ecb9bb
ecb9bb
Reported-By: Robert Kliewer <robert.kliewer@gmail.com>
ecb9bb
Closes: bug #43601
ecb9bb
---
ecb9bb
 grub-core/commands/verify.c | 72 +++++++++++++++++++++++++++++++++++++--------
ecb9bb
 1 file changed, 60 insertions(+), 12 deletions(-)
ecb9bb
ecb9bb
diff --git a/grub-core/commands/verify.c b/grub-core/commands/verify.c
ecb9bb
index 525bdd1..d599576 100644
ecb9bb
--- a/grub-core/commands/verify.c
ecb9bb
+++ b/grub-core/commands/verify.c
ecb9bb
@@ -33,6 +33,13 @@
ecb9bb
 
ecb9bb
 GRUB_MOD_LICENSE ("GPLv3+");
ecb9bb
 
ecb9bb
+struct grub_verified
ecb9bb
+{
ecb9bb
+  grub_file_t file;
ecb9bb
+  void *buf;
ecb9bb
+};
ecb9bb
+typedef struct grub_verified *grub_verified_t;
ecb9bb
+
ecb9bb
 enum
ecb9bb
   {
ecb9bb
     OPTION_SKIP_SIG = 0
ecb9bb
@@ -802,19 +809,39 @@ grub_cmd_verify_signature (grub_extcmd_context_t ctxt,
ecb9bb
 
ecb9bb
 static int sec = 0;
ecb9bb
 
ecb9bb
+static void
ecb9bb
+verified_free (grub_verified_t verified)
ecb9bb
+{
ecb9bb
+  if (verified)
ecb9bb
+    {
ecb9bb
+      grub_free (verified->buf);
ecb9bb
+      grub_free (verified);
ecb9bb
+    }
ecb9bb
+}
ecb9bb
+
ecb9bb
 static grub_ssize_t
ecb9bb
 verified_read (struct grub_file *file, char *buf, grub_size_t len)
ecb9bb
 {
ecb9bb
-  grub_memcpy (buf, (char *) file->data + file->offset, len);
ecb9bb
+  grub_verified_t verified = file->data;
ecb9bb
+
ecb9bb
+  grub_memcpy (buf, (char *) verified->buf + file->offset, len);
ecb9bb
   return len;
ecb9bb
 }
ecb9bb
 
ecb9bb
 static grub_err_t
ecb9bb
 verified_close (struct grub_file *file)
ecb9bb
 {
ecb9bb
-  grub_free (file->data);
ecb9bb
+  grub_verified_t verified = file->data;
ecb9bb
+
ecb9bb
+  grub_file_close (verified->file);
ecb9bb
+  verified_free (verified);
ecb9bb
   file->data = 0;
ecb9bb
-  return GRUB_ERR_NONE;
ecb9bb
+
ecb9bb
+  /* device and name are freed by parent */
ecb9bb
+  file->device = 0;
ecb9bb
+  file->name = 0;
ecb9bb
+
ecb9bb
+  return grub_errno;
ecb9bb
 }
ecb9bb
 
ecb9bb
 struct grub_fs verified_fs =
ecb9bb
@@ -832,6 +859,7 @@ grub_pubkey_open (grub_file_t io, const char *filename)
ecb9bb
   grub_err_t err;
ecb9bb
   grub_file_filter_t curfilt[GRUB_FILE_FILTER_MAX];
ecb9bb
   grub_file_t ret;
ecb9bb
+  grub_verified_t verified;
ecb9bb
 
ecb9bb
   if (!sec)
ecb9bb
     return io;
ecb9bb
@@ -857,7 +885,10 @@ grub_pubkey_open (grub_file_t io, const char *filename)
ecb9bb
 
ecb9bb
   ret = grub_malloc (sizeof (*ret));
ecb9bb
   if (!ret)
ecb9bb
-    return NULL;
ecb9bb
+    {
ecb9bb
+      grub_file_close (sig);
ecb9bb
+      return NULL;
ecb9bb
+    }
ecb9bb
   *ret = *io;
ecb9bb
 
ecb9bb
   ret->fs = &verified_fs;
ecb9bb
@@ -866,29 +897,46 @@ grub_pubkey_open (grub_file_t io, const char *filename)
ecb9bb
     {
ecb9bb
       grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
ecb9bb
 		  "big file signature isn't implemented yet");
ecb9bb
+      grub_file_close (sig);
ecb9bb
+      grub_free (ret);
ecb9bb
+      return NULL;
ecb9bb
+    }
ecb9bb
+  verified = grub_malloc (sizeof (*verified));
ecb9bb
+  if (!verified)
ecb9bb
+    {
ecb9bb
+      grub_file_close (sig);
ecb9bb
+      grub_free (ret);
ecb9bb
       return NULL;
ecb9bb
     }
ecb9bb
-  ret->data = grub_malloc (ret->size);
ecb9bb
-  if (!ret->data)
ecb9bb
+  verified->buf = grub_malloc (ret->size);
ecb9bb
+  if (!verified->buf)
ecb9bb
     {
ecb9bb
+      grub_file_close (sig);
ecb9bb
+      grub_free (verified);
ecb9bb
       grub_free (ret);
ecb9bb
       return NULL;
ecb9bb
     }
ecb9bb
-  if (grub_file_read (io, ret->data, ret->size) != (grub_ssize_t) ret->size)
ecb9bb
+  if (grub_file_read (io, verified->buf, ret->size) != (grub_ssize_t) ret->size)
ecb9bb
     {
ecb9bb
       if (!grub_errno)
ecb9bb
 	grub_error (GRUB_ERR_FILE_READ_ERROR, N_("premature end of file %s"),
ecb9bb
 		    filename);
ecb9bb
+      grub_file_close (sig);
ecb9bb
+      verified_free (verified);
ecb9bb
+      grub_free (ret);
ecb9bb
       return NULL;
ecb9bb
     }
ecb9bb
 
ecb9bb
-  err = grub_verify_signature_real (ret->data, ret->size, 0, sig, NULL);
ecb9bb
+  err = grub_verify_signature_real (verified->buf, ret->size, 0, sig, NULL);
ecb9bb
   grub_file_close (sig);
ecb9bb
   if (err)
ecb9bb
-    return NULL;
ecb9bb
-  io->device = 0;
ecb9bb
-  io->name = 0;
ecb9bb
-  grub_file_close (io);
ecb9bb
+    {
ecb9bb
+      verified_free (verified);
ecb9bb
+      grub_free (ret);
ecb9bb
+      return NULL;
ecb9bb
+    }
ecb9bb
+  verified->file = io;
ecb9bb
+  ret->data = verified;
ecb9bb
   return ret;
ecb9bb
 }
ecb9bb
 
ecb9bb
-- 
ecb9bb
2.4.3
ecb9bb