From 6230b3eeab52b132d5b7e4e164380389325db040 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Wed, 19 Jan 2022 13:44:10 +0000 Subject: lib/commit: always validate metadata This tweaks commit logic in order to always validate metadata, including on commits where the expected checksum is already known. --- src/libostree/ostree-repo-commit.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index e2c86d96..a5aa63b0 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -2478,12 +2478,8 @@ ostree_repo_write_metadata (OstreeRepo *self, normalized = g_variant_get_normal_form (object); } - /* For untrusted objects, verify their structure here */ - if (expected_checksum) - { - if (!_ostree_validate_structureof_metadata (objtype, object, error)) - return FALSE; - } + if (!_ostree_validate_structureof_metadata (objtype, object, error)) + return FALSE; g_autoptr(GBytes) vdata = g_variant_get_data_as_bytes (normalized); if (!write_metadata_object (self, objtype, expected_checksum, -- cgit v1.2.1 From da72c245f4b730d2ff41db996ec14a7f21f097e9 Mon Sep 17 00:00:00 2001 From: Luca BRUNO 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