grub-devel.gnu.org archive mirror
 help / color / mirror / Atom feed
From: Jon McCune <jonmccune@google.com>
To: grub-devel@gnu.org
Cc: Jon McCune <jonmccune@google.com>
Subject: [PATCH v1 2/2] introduce new commands load_env_untrusted and save_env_untrusted:
Date: Mon, 26 Aug 2013 12:29:06 -0700	[thread overview]
Message-ID: <1377545346-23642-3-git-send-email-jonmccune@google.com> (raw)
In-Reply-To: <1377545346-23642-1-git-send-email-jonmccune@google.com>

These commands will load / store a single named environment variable
from / to a 1024-byte file, regardless of the value of check_signatures.

Signed-off-by: Jon McCune <jonmccune@google.com>
---
 grub-core/commands/loadenv.c | 284 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 284 insertions(+)

diff --git a/grub-core/commands/loadenv.c b/grub-core/commands/loadenv.c
index 691d163..f6a363c 100644
--- a/grub-core/commands/loadenv.c
+++ b/grub-core/commands/loadenv.c
@@ -26,6 +26,7 @@
 #include <grub/partition.h>
 #include <grub/lib/envblk.h>
 #include <grub/extcmd.h>
+#include <grub/command.h>
 #include <grub/i18n.h>
 
 GRUB_MOD_LICENSE ("GPLv3+");
@@ -379,7 +380,280 @@ grub_cmd_save_env (grub_extcmd_context_t ctxt, int argc, char **args)
   return grub_errno;
 }
 
+/* It is an error for a file being read or written with load_env_untrusted or
+   save_env_untrusted to be any other size. */
+#define UNTRUSTED_VAR_FILE_SIZE 1024
+
+/* Returns 0 if c is not in the set [A-Za-z0-9_] */
+static int
+is_AZaz09_(int c)
+{
+  return (grub_isalpha(c) || grub_isdigit(c) || '_' == c);
+}
+
+/* Opens 'filename' with all filters disabled (in particular, the PUBKEY filter
+   that insists upon properly signed files, and any kind of compression, since
+   we don't want untrusted input being processed by complex compression
+   algorithms). */
+static grub_file_t
+open_file_untrusted (char *filename)
+{
+  grub_file_filter_t curfilt[GRUB_FILE_FILTER_MAX];
+  grub_file_t file;
+
+  /* Temporarily disable all filters so as to read the untrusted file */
+  grub_memcpy (curfilt, grub_file_filters_enabled, sizeof (curfilt));
+  grub_file_filter_disable_all();
+  file = grub_file_open(filename);
+  /* Restore filters */
+  grub_memcpy (grub_file_filters_enabled, curfilt, sizeof (curfilt));
+
+  return file;
+}
+
+/* Reads an already-opened file into a freshly allocated buffer of size
+   UNTRUSTED_VAR_FILE_SIZE.  Returns pointer to buffer, or NULL on error.  The
+   returned data is *not* guaranteed to be NULL-terminated.
+   TODO: Reduce duplication of logic with read_envblk_file(). (Requires
+   refactoring read_envblk_file() and first-pass goal is to keep changes as
+   self-contained as possible.) */
+static char*
+read_file_untrusted (grub_file_t file)
+{
+  char *buf = NULL;
+  grub_size_t size;
+  grub_off_t offset;
+
+  if (! file)
+    return NULL;
+
+  /* Read file's size, allocate space, and read the file's contents */
+  size = grub_file_size(file);
+  if (size != UNTRUSTED_VAR_FILE_SIZE)
+    {
+      grub_dprintf ("crypt", "ERROR: size %d != %d\n", size,
+                    UNTRUSTED_VAR_FILE_SIZE);
+      return NULL;
+    }
+
+  buf = grub_malloc (size);
+  if (! buf)
+    return NULL;
+
+  grub_dprintf ("crypt", "malloc succeeded\n");
+
+  offset = 0;
+  while (size > 0)
+    {
+      grub_ssize_t ret = grub_file_read (file, buf + offset, size);
+      if (ret == 0)
+        break;
+      else if (ret < 0)
+        {
+          grub_free (buf);
+          return NULL;
+        }
+
+      size -= ret;
+      offset += ret;
+    }
+  grub_dprintf ("crypt", "successfully read %llu bytes from file!\n", offset);
+
+  if (offset != UNTRUSTED_VAR_FILE_SIZE)
+    {
+      grub_dprintf ("crypt", "ERROR read too few bytes (%llu instead of %d) from "
+                    "file!\n", offset, UNTRUSTED_VAR_FILE_SIZE);
+      grub_free (buf);
+      return NULL;
+    }
+
+  return buf;
+}
+
+/* NULL-terminates buf by changing all characters after the first run of 0 or
+   more characters in the set [A-Za-z0-9_].  Assumes buf points to at least
+   UNTRUSTED_VAR_FILE_SIZE bytes. */
+static void
+sanitize_untrusted_string (char buf[UNTRUSTED_VAR_FILE_SIZE])
+{
+  grub_size_t i;
+
+  /* First, NULL-terminate the input for safety */
+  buf[UNTRUSTED_VAR_FILE_SIZE - 1] = '\0';
+  /* Now nullify all characters not in [A-Za-z0-9_] */
+  for (i = 0; i < UNTRUSTED_VAR_FILE_SIZE; i++)
+    if (!is_AZaz09_(buf[i]))
+        buf[i] = '\0';
+}
+
+/* Now that we have read the untrusted data, assign it to the requested
+   environment variable.  It is *not* an error for the data to be a string of
+   length 0 (i.e., for buf[0] == '\0'), it just results in an empty
+   environment variable. */
+static grub_err_t
+grub_cmd_load_env_untrusted (grub_command_t cmd  __attribute__ ((unused)),
+                             int argc, char **argv)
+{
+  grub_file_t file;
+  grub_err_t err;
+  char *buf;
+  unsigned int i;
+
+  grub_dprintf ("crypt", "load_env_untrusted\n");
+
+  if (argc != 2)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments expected"));
+
+  for (i = 0; argv[0][i] != '\0'; i++)
+    if (! is_AZaz09_ (argv[0][i]))
+      return grub_error (GRUB_ERR_BAD_ARGUMENT,
+                         N_("VARIABLE_NAME must be in set [A-Za-z0-9_]"));
+
+  grub_dprintf ("crypt", "variable_name: %s\n", argv[0]);
+  grub_dprintf ("crypt", "file: %s\n", argv[1]);
+
+  file = open_file_untrusted(argv[1]);
+  if (! file)
+    {
+      grub_dprintf ("crypt", "ERROR: couldn't open file: %s\n", argv[1]);
+      return grub_errno;
+    }
+
+  buf = read_file_untrusted(file);
+  grub_file_close (file);
+
+  if (NULL == buf)
+    return GRUB_ERR_FILE_READ_ERROR;
+
+  sanitize_untrusted_string(buf);
+
+  err = grub_env_set(argv[0], buf);
+
+  if (buf)
+    grub_free (buf);
+  return err;
+}
+
+/* Overwrites the contents of an existing 1024-byte file with the first run of
+   bytes from the specified environment variable that comply with [A-Za-z0-9_].
+   Two arguments: the name of the environment variable, and the name of the file
+   into which that variable's contents should be written.  Extra space in the
+   file is padded out with a single '\n' followed by '#' characters. */
+static grub_err_t
+grub_cmd_save_env_untrusted (grub_command_t cmd  __attribute__ ((unused)),
+                             int argc, char **argv)
+{
+  grub_file_t file;
+  const char* var_contents;
+  grub_size_t var_contents_len;
+  char* old_file_contents;
+  grub_err_t err;
+  char *new_file_contents;
+  struct grub_cmd_save_env_ctx ctx = {
+    .head = 0,
+    .tail = 0
+  };
+  grub_disk_t disk;
+  grub_disk_addr_t part_start;
+  struct blocklist *p;
+  grub_size_t index;
+
+  grub_dprintf ("crypt", "save_env_untrusted\n");
+
+  if (argc != 2)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments expected"));
+
+  for (index = 0; argv[0][index] != '\0'; index++)
+    if (! is_AZaz09_ (argv[0][index]))
+      return grub_error (GRUB_ERR_BAD_ARGUMENT,
+                         N_("VARIABLE_NAME must be in set [A-Za-z0-9_]"));
+
+  var_contents = grub_env_get(argv[0]);
+  if (NULL == var_contents)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("no such variable"));
+  /* This ensures var_contents is NULL-terminated for processing below */
+  var_contents_len = grub_strlen (var_contents);  /* TODO: get strnlen */
+  if (var_contents_len >= UNTRUSTED_VAR_FILE_SIZE)
+    return grub_error (GRUB_ERR_OUT_OF_RANGE, N_("variable value too long"));
+
+  /* Make sure the variable only contains characters in [A-Za-z0-9_] */
+  for (index = 0; index < var_contents_len; index++)
+    if (!is_AZaz09_ (var_contents[index]))
+      return grub_error (GRUB_ERR_OUT_OF_RANGE,
+                         N_("illegal characters in variable"));
+
+  file = open_file_untrusted(argv[1]);
+  if (! file)
+    return grub_errno;
+
+  grub_dprintf ("crypt", "save_env_untrusted opened file %s for reading\n",
+                argv[1]);
+
+  if (! file->device->disk)
+    {
+      grub_file_close (file);
+      return grub_error (GRUB_ERR_BAD_DEVICE, "disk device required");
+    }
+
+  file->read_hook = save_env_read_hook;
+  file->read_hook_data = &ctx;
+  old_file_contents = read_file_untrusted(file);
+  file->read_hook = 0;
+  if (NULL == old_file_contents)
+    {
+      err = grub_error (GRUB_ERR_FILE_READ_ERROR,
+                        N_("Unable to read existing file blocks"));
+      goto fail;
+    }
+
+  grub_dprintf ("crypt", "read file, blocklist is built.\n");
+
+  err = check_blocklists (old_file_contents, ctx.head, file);
+  if (GRUB_ERR_NONE != err)
+    goto fail;
+
+  grub_dprintf ("crypt", "blocklist checked successfully.\n");
+
+  new_file_contents = grub_malloc (UNTRUSTED_VAR_FILE_SIZE);
+  if (NULL == new_file_contents)
+    {
+      err = grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("malloc failed"));
+      goto fail;
+    }
+
+  grub_memcpy(new_file_contents, old_file_contents, UNTRUSTED_VAR_FILE_SIZE);
+  grub_strncpy(new_file_contents, var_contents, UNTRUSTED_VAR_FILE_SIZE);
+  new_file_contents[var_contents_len] = '\n';
+  for (index = var_contents_len + 1; index < UNTRUSTED_VAR_FILE_SIZE; index++)
+    new_file_contents[index] = '#';  /* backfill file with '#' character */
+
+  grub_dprintf ("crypt", "ready to write: ---\n%s\n---\n", new_file_contents);
+
+  /* TODO: Refactor write_blocklists() so we can just call it here instead. */
+  disk = file->device->disk;
+  part_start = grub_partition_get_start (disk->partition);
+
+  index = 0;
+  for (p = ctx.head; p; index += p->length, p = p->next)
+    {
+      err = grub_disk_write (disk, p->sector - part_start,
+                             p->offset, p->length, new_file_contents + index);
+      if (GRUB_ERR_NONE != err)
+        goto fail;
+    }
+
+fail:
+  if (old_file_contents)
+    grub_free (old_file_contents);
+  if (new_file_contents)
+    grub_free (new_file_contents);
+  free_blocklists (ctx.head);
+  grub_file_close (file);
+  return err;
+}
+
 static grub_extcmd_t cmd_load, cmd_list, cmd_save;
+static grub_command_t cmd_load_env_untrusted, cmd_save_env_untrusted;
 
 GRUB_MOD_INIT(loadenv)
 {
@@ -396,6 +670,14 @@ GRUB_MOD_INIT(loadenv)
 			  N_("[-f FILE] variable_name [...]"),
 			  N_("Save variables to environment block file."),
 			  options);
+  cmd_load_env_untrusted =
+    grub_register_command ("load_env_untrusted", grub_cmd_load_env_untrusted,
+              N_("VARIABLE_NAME FILE"),
+              N_("Assign the contents of FILE to VARIABLE_NAME"));
+  cmd_save_env_untrusted =
+    grub_register_command ("save_env_untrusted", grub_cmd_save_env_untrusted,
+              N_("VARIABLE_NAME FILE"),
+              N_("Write the contents of VARIABLE_NAME into pre-existing FILE"));
 }
 
 GRUB_MOD_FINI(loadenv)
@@ -403,4 +685,6 @@ GRUB_MOD_FINI(loadenv)
   grub_unregister_extcmd (cmd_load);
   grub_unregister_extcmd (cmd_list);
   grub_unregister_extcmd (cmd_save);
+  grub_unregister_command (cmd_load_env_untrusted);
+  grub_unregister_command (cmd_save_env_untrusted);
 }
-- 
1.8.3



      parent reply	other threads:[~2013-08-26 19:29 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-26 19:29 [PATCH v1 0/2] New commands: load_env_untrusted, save_env_untrusted Jon McCune
2013-08-26 19:29 ` [PATCH v1 1/2] make check_blocklists() not depend on an envblk data type Jon McCune
2013-08-26 19:29 ` Jon McCune [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1377545346-23642-3-git-send-email-jonmccune@google.com \
    --to=jonmccune@google.com \
    --cc=grub-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).