Merge pull request ostreedev#3261 from cgwalters/validate-xattrs

core: Validate that xattr names aren't empty

src/libostree/ostree-core-private.h

@@ -90,6 +90,7 @@ void _ostree_gfileinfo_to_stbuf (GFileInfo *file_info, struct stat *out_stbuf);
 gboolean _ostree_gfileinfo_equal (GFileInfo *a, GFileInfo *b);
 gboolean _ostree_stbuf_equal (struct stat *stbuf_a, struct stat *stbuf_b);
 GFileInfo *_ostree_mode_uidgid_to_gfileinfo (mode_t mode, uid_t uid, gid_t gid);
+gboolean _ostree_validate_structureof_xattrs (GVariant *xattrs, GError **error);
 
 static inline void
 _ostree_checksum_inplace_from_bytes_v (GVariant *csum_v, char *buf)

src/libostree/ostree-core.c

@@ -2277,6 +2277,27 @@ ostree_validate_structureof_file_mode (guint32 mode, GError **error)
   return TRUE;
 }
 
+/* Currently ostree imposes no restrictions on xattrs on its own;
+ * they can e.g. be arbitrariliy sized or in number.
+ * However, we do validate the key is non-empty, as that is known
+ * to always fail.
+ */
+gboolean
+_ostree_validate_structureof_xattrs (GVariant *xattrs, GError **error)
+{
+  const guint n = g_variant_n_children (xattrs);
+  for (guint i = 0; i < n; i++)
+    {
+      const guint8 *name;
+      g_autoptr (GVariant) value = NULL;
+      g_variant_get_child (xattrs, i, "(^&ay@ay)", &name, &value);
+      if (!*name)
+        return glnx_throw (error, "Invalid xattr name (empty or missing NUL) index=%d", i);
+      i++;
+    }
+  return TRUE;
+}
+
 /**
  * ostree_validate_structureof_dirmeta:
  * @dirmeta: A dirmeta object, %OSTREE_OBJECT_TYPE_DIR_META
@@ -2303,6 +2324,10 @@ ostree_validate_structureof_dirmeta (GVariant *dirmeta, GError **error)
   if (!validate_stat_mode_perms (mode, error))
     return FALSE;
 
+  g_autoptr (GVariant) xattrs = g_variant_get_child_value (dirmeta, 3);
+  if (!_ostree_validate_structureof_xattrs (xattrs, error))
+    return FALSE;
+
   return TRUE;
 }
 

src/libostree/ostree-repo-checkout.c

@@ -1123,7 +1123,7 @@ checkout_tree_at_recurse (OstreeRepo *self, OstreeRepoCheckoutAtOptions *options
   if (!did_exist && xattrs)
     {
       if (!glnx_fd_set_all_xattrs (destination_dfd, xattrs, cancellable, error))
-        return FALSE;
+        return glnx_prefix_error (error, "Processing dirmeta %s", dirmeta_checksum);
     }
 
   /* Process files in this subdir */

src/libostree/ostree-repo-commit.c

@@ -139,6 +139,8 @@ gboolean
 _ostree_write_bareuser_metadata (int fd, guint32 uid, guint32 gid, guint32 mode, GVariant *xattrs,
                                  GError **error)
 {
+  if (xattrs != NULL && !_ostree_validate_structureof_xattrs (xattrs, error))
+    return FALSE;
   g_autoptr (GVariant) filemeta = create_file_metadata (uid, gid, mode, xattrs);
 
   if (TEMP_FAILURE_RETRY (fsetxattr (fd, "user.ostreemeta", (char *)g_variant_get_data (filemeta),

tests/test-basic-c.c

@@ -513,6 +513,23 @@ test_read_xattrs (void)
   }
 }
 
+static void
+test_dirmeta_xattrs (void)
+{
+  g_autoptr (GError) local_error = NULL;
+  GError **error = &local_error;
+  const guint32 uidgid = GUINT32_TO_BE (42);
+  const guint32 mode = GUINT32_TO_BE (S_IFDIR | S_IRWXU);
+  g_autoptr (GVariantBuilder) xattr_builder = g_variant_builder_new (G_VARIANT_TYPE ("a(ayay)"));
+  const char *data = "data";
+  g_variant_builder_add (xattr_builder, "(@ay@ay)", g_variant_new_bytestring (""),
+                         g_variant_new_bytestring (data));
+  g_autoptr (GVariant) dirmeta = g_variant_new ("(uuu@a(ayay))", uidgid, uidgid, mode,
+                                                g_variant_builder_end (xattr_builder));
+  g_assert (!ostree_validate_structureof_dirmeta (dirmeta, error));
+  g_assert_error (local_error, G_IO_ERROR, G_IO_ERROR_FAILED);
+}
+
 int
 main (int argc, char **argv)
 {
@@ -533,6 +550,7 @@ main (int argc, char **argv)
   g_test_add_func ("/remotename", test_validate_remotename);
   g_test_add_func ("/big-metadata", test_big_metadata);
   g_test_add_func ("/read-xattrs", test_read_xattrs);
+  g_test_add_func ("/dirmeta-xattrs", test_dirmeta_xattrs);
 
   return g_test_run ();
 out: