Limit metadata to 10 MiB

If fetching GPG-signed commits over plain HTTP, a MitM attacker can
fill up the drive of targets by simply returning an enormous stream
for the commit object.

Related to this, an attacker can also cause OSTree to perform large
memory allocations by returning enormous GVariants in the metadata.

This helps close that attack by limiting all metadata objects to 10
MiB, so the initial fetch will be truncated.

But now the attack is only slightly more difficult as the attacker
will have to return a correctly formed commit object, then return a
large stream of < 10 MiB dirmeta/dirtree objects.

https://bugzilla.gnome.org/show_bug.cgi?id=725921

8 files changed, 221 insertions, 31 deletions

diff --git a/Makefile-tests.am b/Makefile-tests.am
index 13c5d60d..5c868410 100644
--- a/Makefile-tests.am
+++ b/Makefile-tests.am
@@ -29,6 +29,7 @@ testfiles = test-basic \
 	test-libarchive \
 	test-pull-archive-z \
 	test-pull-corruption \
+	test-pull-large-metadata \
 	test-pull-resume \
 	test-gpg-signed-commit \
 	test-admin-deploy-syslinux \
diff --git a/src/libostree/ostree-core.h b/src/libostree/ostree-core.h
index ced4ff4b..a867dbe4 100644
--- a/src/libostree/ostree-core.h
+++ b/src/libostree/ostree-core.h
@@ -29,9 +29,19 @@ G_BEGIN_DECLS
 /**
  * OSTREE_MAX_METADATA_SIZE:
  * 
- * Maximum permitted size in bytes of metadata objects.
+ * Maximum permitted size in bytes of metadata objects.  This is an
+ * arbitrary number, but really, no one should be putting humongous
+ * data in metadata.
  */
-#define OSTREE_MAX_METADATA_SIZE (1 << 26)
+#define OSTREE_MAX_METADATA_SIZE (10 * 1024 * 1024)
+
+/**
+ * OSTREE_MAX_METADATA_WARN_SIZE:
+ * 
+ * Objects committed above this size will be allowed, but a warning
+ * will be emitted.
+ */
+#define OSTREE_MAX_METADATA_WARN_SIZE (7 * 1024 * 1024)
 
 /**
  * OSTREE_MAX_RECURSION:
diff --git a/src/libostree/ostree-fetcher.c b/src/libostree/ostree-fetcher.c
index 73140988..92dd0a04 100644
--- a/src/libostree/ostree-fetcher.c
+++ b/src/libostree/ostree-fetcher.c
@@ -52,6 +52,8 @@ typedef struct {
   GFile *out_tmpfile;
   GOutputStream *out_stream;
 
+  guint64 max_size;
+  guint64 current_size;
   guint64 content_length;
 
   GCancellable *cancellable;
@@ -260,22 +262,21 @@ ostree_fetcher_queue_pending_uri (OstreeFetcher *self,
   ostree_fetcher_process_pending_queue (self);
 }
 
-static void
-on_splice_complete (GObject        *object,
-                    GAsyncResult   *result,
-                    gpointer        user_data) 
+static gboolean
+finish_stream (OstreeFetcherPendingURI *pending,
+               GCancellable            *cancellable,
+               GError                 **error)
 {
-  OstreeFetcherPendingURI *pending = user_data;
-  gs_unref_object GFileInfo *file_info = NULL;
+  gboolean ret = FALSE;
   goffset filesize;
-  GError *local_error = NULL;
+  gs_unref_object GFileInfo *file_info = NULL;
 
   /* Close it here since we do an async fstat(), where we don't want
    * to hit a bad fd.
    */
   if (pending->out_stream)
     {
-      if (!g_output_stream_close (pending->out_stream, pending->cancellable, &local_error))
+      if (!g_output_stream_close (pending->out_stream, pending->cancellable, error))
         goto out;
       g_hash_table_remove (pending->self->output_stream_set, pending->out_stream);
     }
@@ -283,7 +284,7 @@ on_splice_complete (GObject        *object,
   pending->state = OSTREE_FETCHER_STATE_COMPLETE;
   file_info = g_file_query_info (pending->out_tmpfile, OSTREE_GIO_FAST_QUERYINFO,
                                  G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS,
-                                 pending->cancellable, &local_error);
+                                 pending->cancellable, error);
   if (!file_info)
     goto out;
 
@@ -296,7 +297,7 @@ on_splice_complete (GObject        *object,
   filesize = g_file_info_get_size (file_info);
   if (filesize < pending->content_length)
     {
-      g_set_error (&local_error, G_IO_ERROR, G_IO_ERROR_FAILED, "Download incomplete");
+      g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Download incomplete");
       goto out;
     }
   else
@@ -304,12 +305,107 @@ on_splice_complete (GObject        *object,
       pending->self->total_downloaded += g_file_info_get_size (file_info);
     }
 
+  ret = TRUE;
  out:
   (void) g_input_stream_close (pending->request_body, NULL, NULL);
+  return ret;
+}
+
+static void
+on_stream_read (GObject        *object,
+                GAsyncResult   *result,
+                gpointer        user_data);
+
+static void
+on_out_splice_complete (GObject        *object,
+                        GAsyncResult   *result,
+                        gpointer        user_data) 
+{
+  OstreeFetcherPendingURI *pending = user_data;
+  gssize bytes_written;
+  GError *local_error = NULL;
+  GError **error = &local_error;
+
+  bytes_written = g_output_stream_splice_finish ((GOutputStream *)object,
+                                                 result,
+                                                 error);
+  if (bytes_written < 0)
+    goto out;
+
+  g_input_stream_read_bytes_async (pending->request_body, 8192, G_PRIORITY_DEFAULT,
+                                   pending->cancellable, on_stream_read, pending);
+
+ out:
   if (local_error)
-    g_simple_async_result_take_error (pending->result, local_error);
-  g_simple_async_result_complete (pending->result);
-  g_object_unref (pending->result);
+    {
+      g_simple_async_result_take_error (pending->result, local_error);
+      g_simple_async_result_complete (pending->result);
+    }
+}
+
+static void
+on_stream_read (GObject        *object,
+                GAsyncResult   *result,
+                gpointer        user_data) 
+{
+  OstreeFetcherPendingURI *pending = user_data;
+  gs_unref_bytes GBytes *bytes = NULL;
+  gsize bytes_read;
+  GError *local_error = NULL;
+  GError **error = &local_error;
+
+  bytes = g_input_stream_read_bytes_finish ((GInputStream*)object, result, error);
+  if (!bytes)
+    goto out;
+
+  bytes_read = g_bytes_get_size (bytes);
+  if (bytes_read == 0)
+    {
+      if (!finish_stream (pending, pending->cancellable, error))
+        goto out;
+      g_simple_async_result_complete (pending->result);
+      g_object_unref (pending->result);
+    }
+  else
+    {
+      if (pending->max_size > 0)
+        {
+          if (bytes_read > pending->max_size ||
+              (bytes_read + pending->current_size) > pending->max_size)
+            {
+              gs_free char *uristr = soup_uri_to_string (pending->uri, FALSE);
+              g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                           "URI %s exceeded maximum size of %" G_GUINT64_FORMAT " bytes",
+                           uristr,
+                           pending->max_size);
+              goto out;
+            }
+        }
+      
+      pending->current_size += bytes_read;
+
+      /* We do this instead of _write_bytes_async() as that's not
+       * guaranteed to do a complete write.
+       */
+      {
+        gs_unref_object GInputStream *membuf =
+          g_memory_input_stream_new_from_bytes (bytes);
+        g_output_stream_splice_async (pending->out_stream, membuf,
+                                      G_OUTPUT_STREAM_SPLICE_CLOSE_SOURCE,
+                                      G_PRIORITY_DEFAULT,
+                                      pending->cancellable,
+                                      on_out_splice_complete,
+                                      pending);
+      }
+    }
+
+ out:
+  if (local_error)
+    {
+      g_simple_async_result_take_error (pending->result, local_error);
+      g_simple_async_result_complete (pending->result);
+      g_object_unref (pending->result);
+    }
 }
 
 static void
@@ -320,7 +416,6 @@ on_request_sent (GObject        *object,
   OstreeFetcherPendingURI *pending = user_data;
   GError *local_error = NULL;
   gs_unref_object SoupMessage *msg = NULL;
-  GOutputStreamSpliceFlags flags = G_OUTPUT_STREAM_SPLICE_CLOSE_SOURCE;
 
   pending->state = OSTREE_FETCHER_STATE_COMPLETE;
   pending->request_body = soup_request_send_finish ((SoupRequest*) object,
@@ -371,8 +466,8 @@ on_request_sent (GObject        *object,
       if (!pending->out_stream)
         goto out;
       g_hash_table_add (pending->self->output_stream_set, g_object_ref (pending->out_stream));
-      g_output_stream_splice_async (pending->out_stream, pending->request_body, flags, G_PRIORITY_DEFAULT,
-                                    pending->cancellable, on_splice_complete, pending);
+      g_input_stream_read_bytes_async (pending->request_body, 8192, G_PRIORITY_DEFAULT,
+                                       pending->cancellable, on_stream_read, pending);
       
     }
   else
@@ -394,6 +489,7 @@ static OstreeFetcherPendingURI *
 ostree_fetcher_request_uri_internal (OstreeFetcher         *self,
                                      SoupURI               *uri,
                                      gboolean               is_stream,
+                                     guint64                max_size,
                                      GCancellable          *cancellable,
                                      GAsyncReadyCallback    callback,
                                      gpointer               user_data,
@@ -406,6 +502,7 @@ ostree_fetcher_request_uri_internal (OstreeFetcher         *self,
   pending->refcount = 1;
   pending->self = g_object_ref (self);
   pending->uri = soup_uri_copy (uri);
+  pending->max_size = max_size;
   pending->is_stream = is_stream;
   if (!is_stream)
     {
@@ -433,6 +530,7 @@ ostree_fetcher_request_uri_internal (OstreeFetcher         *self,
 void
 ostree_fetcher_request_uri_with_partial_async (OstreeFetcher         *self,
                                                SoupURI               *uri,
+                                               guint64                max_size,
                                                GCancellable          *cancellable,
                                                GAsyncReadyCallback    callback,
                                                gpointer               user_data)
@@ -443,7 +541,7 @@ ostree_fetcher_request_uri_with_partial_async (OstreeFetcher         *self,
 
   self->total_requests++;
 
-  pending = ostree_fetcher_request_uri_internal (self, uri, FALSE, cancellable,
+  pending = ostree_fetcher_request_uri_internal (self, uri, FALSE, max_size, cancellable,
                                                  callback, user_data,
                                                  ostree_fetcher_request_uri_with_partial_async);
 
@@ -496,6 +594,7 @@ ostree_fetcher_request_uri_with_partial_finish (OstreeFetcher         *self,
 void
 ostree_fetcher_stream_uri_async (OstreeFetcher         *self,
                                  SoupURI               *uri,
+                                 guint64                max_size,
                                  GCancellable          *cancellable,
                                  GAsyncReadyCallback    callback,
                                  gpointer               user_data)
@@ -504,7 +603,7 @@ ostree_fetcher_stream_uri_async (OstreeFetcher         *self,
 
   self->total_requests++;
 
-  pending = ostree_fetcher_request_uri_internal (self, uri, TRUE, cancellable,
+  pending = ostree_fetcher_request_uri_internal (self, uri, TRUE, max_size, cancellable,
                                                  callback, user_data,
                                                  ostree_fetcher_stream_uri_async);
 
diff --git a/src/libostree/ostree-fetcher.h b/src/libostree/ostree-fetcher.h
index 928d2a35..55b5e1d2 100644
--- a/src/libostree/ostree-fetcher.h
+++ b/src/libostree/ostree-fetcher.h
@@ -65,6 +65,7 @@ guint ostree_fetcher_get_n_requests (OstreeFetcher       *self);
 
 void ostree_fetcher_request_uri_with_partial_async (OstreeFetcher         *self,
                                                     SoupURI               *uri,
+                                                    guint64                max_size,
                                                     GCancellable          *cancellable,
                                                     GAsyncReadyCallback    callback,
                                                     gpointer               user_data);
@@ -74,10 +75,11 @@ GFile *ostree_fetcher_request_uri_with_partial_finish (OstreeFetcher *self,
                                                        GError       **error);
 
 void ostree_fetcher_stream_uri_async (OstreeFetcher         *self,
-                                       SoupURI               *uri,
-                                       GCancellable          *cancellable,
-                                       GAsyncReadyCallback    callback,
-                                       gpointer               user_data);
+                                      SoupURI               *uri,
+                                      guint64                max_size,
+                                      GCancellable          *cancellable,
+                                      GAsyncReadyCallback    callback,
+                                      gpointer               user_data);
 
 GInputStream *ostree_fetcher_stream_uri_finish (OstreeFetcher         *self,
                                                 GAsyncResult          *result,
diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c
index c3bc8368..b9553707 100644
--- a/src/libostree/ostree-repo-commit.c
+++ b/src/libostree/ostree-repo-commit.c
@@ -551,6 +551,20 @@ write_object (OstreeRepo         *self,
                                         cancellable, error))
         goto out;
 
+      if (OSTREE_OBJECT_TYPE_IS_META (objtype))
+        {
+          if (G_UNLIKELY (file_object_length > OSTREE_MAX_METADATA_WARN_SIZE))
+            {
+              gs_free char *metasize = g_format_size (file_object_length);
+              gs_free char *warnsize = g_format_size (OSTREE_MAX_METADATA_WARN_SIZE);
+              gs_free char *maxsize = g_format_size (OSTREE_MAX_METADATA_SIZE);
+              g_warning ("metadata object %s is %s, which is larger than the warning threshold of %s." \
+                         "  The hard limit on metadata size is %s.  Put large content in the tree itself, not in metadata.",
+                         actual_checksum,
+                         metasize, warnsize, maxsize);
+            }
+        }
+
       g_clear_pointer (&temp_filename, g_free);
       g_clear_object (&temp_file);
     }
@@ -1049,16 +1063,35 @@ ostree_repo_write_metadata (OstreeRepo         *self,
                             GCancellable       *cancellable,
                             GError            **error)
 {
+  gboolean ret = FALSE;
   gs_unref_object GInputStream *input = NULL;
   gs_unref_variant GVariant *normalized = NULL;
 
   normalized = g_variant_get_normal_form (object);
+
+  if (G_UNLIKELY (g_variant_get_size (normalized) > OSTREE_MAX_METADATA_SIZE))
+    {
+      gs_free char *input_bytes = g_format_size (g_variant_get_size (normalized));
+      gs_free char *max_bytes = g_format_size (OSTREE_MAX_METADATA_SIZE);
+      g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                   "Metadata object of type '%s' is %s; maximum metadata size is %s",
+                   ostree_object_type_to_string (objtype),
+                   input_bytes,
+                   max_bytes);
+      goto out;
+    }
+
   input = ot_variant_read (normalized);
 
-  return write_object (self, objtype, expected_checksum,
-                       input, g_variant_get_size (normalized),
-                       out_csum,
-                       cancellable, error);
+  if (!write_object (self, objtype, expected_checksum,
+                     input, g_variant_get_size (normalized),
+                     out_csum,
+                     cancellable, error))
+    goto out;
+
+  ret = TRUE;
+ out:
+  return ret;
 }
 
 /**
diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index e3e5a775..9097d011 100644
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -287,7 +287,9 @@ fetch_uri_contents_membuf_sync (OtPullData    *pull_data,
   fetch_data.pull_data = pull_data;
 
   pull_data->fetching_sync_uri = uri;
-  ostree_fetcher_stream_uri_async (pull_data->fetcher, uri, cancellable,
+  ostree_fetcher_stream_uri_async (pull_data->fetcher, uri,
+                                   OSTREE_MAX_METADATA_SIZE,
+                                   cancellable,
                                    fetch_uri_sync_on_complete, &fetch_data);
 
   run_mainloop_monitor_fetcher (pull_data);
@@ -883,7 +885,9 @@ enqueue_one_object_request (OtPullData        *pull_data,
   fetch_data->pull_data = pull_data;
   fetch_data->object = ostree_object_name_serialize (checksum, objtype);
   fetch_data->is_detached_meta = is_detached_meta;
-  ostree_fetcher_request_uri_with_partial_async (pull_data->fetcher, obj_uri, pull_data->cancellable,
+  ostree_fetcher_request_uri_with_partial_async (pull_data->fetcher, obj_uri,
+                                                 is_meta ? OSTREE_MAX_METADATA_SIZE : 0,
+                                                 pull_data->cancellable,
                                                  is_meta ? meta_fetch_on_complete : content_fetch_on_complete, fetch_data);
   soup_uri_free (obj_uri);
 }
diff --git a/tests/test-pull-corruption.sh b/tests/test-pull-corruption.sh
index 9a3cbf55..7e4055c1 100755
--- a/tests/test-pull-corruption.sh
+++ b/tests/test-pull-corruption.sh
@@ -23,7 +23,7 @@ set -e
 
 setup_fake_remote_repo1 "archive-z2"
 
-echo '1..1'
+echo '1..2'
 
 repopath=${test_tmpdir}/ostree-srv/gnomerepo
 cp -a ${repopath} ${repopath}.orig
diff --git a/tests/test-pull-large-metadata.sh b/tests/test-pull-large-metadata.sh
new file mode 100644
index 00000000..9b97b826
--- /dev/null
+++ b/tests/test-pull-large-metadata.sh
@@ -0,0 +1,41 @@
+#!/bin/bash
+#
+# Copyright (C) 2011 Colin Walters <walters@verbum.org>
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the
+# Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+# Boston, MA 02111-1307, USA.
+
+set -e
+
+. $(dirname $0)/libtest.sh
+
+setup_fake_remote_repo1 "archive-z2"
+
+echo '1..1'
+
+# Overwrite the commit object with 20 M of 
+cd ${test_tmpdir}
+rev=$(cd ostree-srv && ostree --repo=gnomerepo rev-parse main)
+dd if=/dev/zero bs=1M count=20 of=ostree-srv/gnomerepo/objects/$(echo $rev | cut -b 1-2)/$(echo $rev | cut -b 3-).commit
+
+cd ${test_tmpdir}
+mkdir repo
+${CMD_PREFIX} ostree --repo=repo init
+${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false origin $(cat httpd-address)/ostree/gnomerepo
+
+if ostree --repo=repo pull origin main 2>pulllog.txt 1>&2; then
+    assert_not_reached "pull unexpectedly succeeded!"
+fi
+assert_file_has_content pulllog.txt "exceeded maximum"