From da72c245f4b730d2ff41db996ec14a7f21f097e9 Mon Sep 17 00:00:00 2001
From: Luca BRUNO <luca.bruno@coreos.com>
Date: Thu, 20 Jan 2022 10:54:30 +0000
Subject: lib/commit: reject empty metadata keys

This adds one more check to the metadata validation logic in order
to reject empty metadata keys.
---
 src/libostree/ostree-core.c    | 13 +++++++++++++
 src/ostree/ot-builtin-commit.c |  9 +++++----
 tests/test-basic-user-only.sh  | 13 ++++++++++++-
 3 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c
index 0abd90a4..038606e9 100644
--- a/src/libostree/ostree-core.c
+++ b/src/libostree/ostree-core.c
@@ -2197,6 +2197,19 @@ ostree_validate_structureof_commit (GVariant      *commit,
   if (!validate_variant (commit, OSTREE_COMMIT_GVARIANT_FORMAT, error))
     return FALSE;
 
+  g_autoptr(GVariant) metadata = NULL;
+  g_variant_get_child (commit, 0, "@a{sv}", &metadata);
+  g_assert (metadata != NULL);
+  g_autoptr(GVariantIter) metadata_iter = g_variant_iter_new (metadata);
+  g_assert (metadata_iter != NULL);
+  g_autoptr(GVariant) metadata_entry = NULL;
+  const gchar *metadata_key = NULL;
+  while (g_variant_iter_loop (metadata_iter, "{sv}", &metadata_key, NULL))
+    {
+      if (metadata_key == NULL || strlen (metadata_key) == 0)
+        return glnx_throw (error, "Empty metadata key");
+    }
+
   g_autoptr(GVariant) parent_csum_v = NULL;
   g_variant_get_child (commit, 1, "@ay", &parent_csum_v);
   gsize n_elts;
diff --git a/src/ostree/ot-builtin-commit.c b/src/ostree/ot-builtin-commit.c
index 845013ed..c43f9b3c 100644
--- a/src/ostree/ot-builtin-commit.c
+++ b/src/ostree/ot-builtin-commit.c
@@ -335,17 +335,18 @@ parse_keyvalue_strings (GVariantBuilder   *builder,
       if (!eq)
         return glnx_throw (error, "Missing '=' in KEY=VALUE metadata '%s'", s);
       g_autofree char *key = g_strndup (s, eq - s);
+      const char *value = eq + 1;
       if (is_gvariant_print)
         {
-          g_autoptr(GVariant) value = g_variant_parse (NULL, eq + 1, NULL, NULL, error);
-          if (!value)
+          g_autoptr(GVariant) variant = g_variant_parse (NULL, value, NULL, NULL, error);
+          if (!variant)
             return glnx_prefix_error (error, "Parsing %s", s);
 
-          g_variant_builder_add (builder, "{sv}", key, value);
+          g_variant_builder_add (builder, "{sv}", key, variant);
         }
       else
         g_variant_builder_add (builder, "{sv}", key,
-                               g_variant_new_string (eq + 1));
+                               g_variant_new_string (value));
     }
 
   return TRUE;
diff --git a/tests/test-basic-user-only.sh b/tests/test-basic-user-only.sh
index 368abf0d..f6e8606d 100755
--- a/tests/test-basic-user-only.sh
+++ b/tests/test-basic-user-only.sh
@@ -23,7 +23,7 @@ set -euo pipefail
 
 mode="bare-user-only"
 setup_test_repository "$mode"
-extra_basic_tests=6
+extra_basic_tests=7
 . $(dirname $0)/basic-test.sh
 
 $CMD_PREFIX ostree --version > version.yaml
@@ -54,6 +54,17 @@ fi
 assert_file_has_content err.txt "Content object.*invalid mode.*with bits 040.*"
 echo "ok failed to commit suid"
 
+cd ${test_tmpdir}
+rm repo-input -rf
+ostree_repo_init repo-input init --mode=archive
+rm files -rf && mkdir files
+if $CMD_PREFIX ostree --repo=repo-input commit -b metadata --tree=dir=files --add-metadata-string='=FOO' 2>err.txt; then
+    assert_not_reached "committed an empty metadata key"
+fi
+assert_file_has_content err.txt "Empty metadata key"
+$CMD_PREFIX ostree --repo=repo-input commit -b metadata --tree=dir=files --add-metadata-string='FOO='
+echo "ok rejected invalid metadata"
+
 cd ${test_tmpdir}
 rm repo-input -rf
 ostree_repo_init repo-input init --mode=archive
-- 
cgit v1.2.1