pull: Fix GLib assertion crash on invalid UTF-8 ref names

When ostree pull or ostree pull-local receives a branch name containing
invalid UTF-8 bytes, g_variant_new_strv() triggers a GLib-CRITICAL
assertion failure on g_utf8_validate(), followed by a NULL pointer
dereference and segmentation fault.

Fixes: https://github.com/ostreedev/ostree/issues/2959

diff --git a/Makefile-tests.am b/Makefile-tests.am
index e62d45d6cd2129082812ab55e29d81a0c27973f3..638a145e960ff8fd6196f573b2c3cf9030792fb4 100644 (file)
--- a/Makefile-tests.am
+++ b/Makefile-tests.am
@@ -292,7 +292,7 @@ _installed_or_uninstalled_test_programs = tests/test-varint tests/test-ot-unix-u
        tests/test-keyfile-utils tests/test-ot-opt-utils tests/test-ot-tool-util \
        tests/test-checksum tests/test-lzma tests/test-rollsum tests/test-bootconfig-parser-internals \
        tests/test-basic-c tests/test-sysroot-c tests/test-pull-c tests/test-repo tests/test-include-ostree-h tests/test-kargs \
-       tests/test-rfc2616-dates tests/test-pem
+       tests/test-rfc2616-dates tests/test-pem tests/test-validate-utf8
 
 if USE_GPGME
 _installed_or_uninstalled_test_programs += \
@@ -435,6 +435,13 @@ tests_test_pem_SOURCES = \
 tests_test_pem_CFLAGS = $(TESTS_CFLAGS)
 tests_test_pem_LDADD = $(TESTS_LDADD)
 
+tests_test_validate_utf8_SOURCES = \
+       src/libostree/ostree-core.c \
+       src/libostree/ostree-varint.c \
+       tests/test-validate-utf8.c
+tests_test_validate_utf8_CFLAGS = $(TESTS_CFLAGS) $(libglnx_cflags)
+tests_test_validate_utf8_LDADD = $(TESTS_LDADD)
+
 noinst_PROGRAMS += tests/test-commit-sign-sh-ext
 tests_test_commit_sign_sh_ext_CFLAGS = $(TESTS_CFLAGS)
 tests_test_commit_sign_sh_ext_LDADD = $(TESTS_LDADD)

diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c
index 3e462838f60384b68fc90f2a8a7e0d2dc6b53d97..12d11421aeb05d1cf58237361eee443f4caba6be 100644 (file)
--- a/src/libostree/ostree-core.c
+++ b/src/libostree/ostree-core.c
@@ -226,6 +226,12 @@ ostree_validate_rev (const char *rev, GError **error)
 {
   g_autoptr (GMatchInfo) match = NULL;
 
+  g_return_val_if_fail (rev != NULL, FALSE);
+
+  /* Validate the string is valid UTF-8 */
+  if (!g_utf8_validate (rev, -1, NULL))
+    return glnx_throw (error, "Invalid ref name: not valid UTF-8");
+
   static gsize regex_initialized;
   static GRegex *regex;
   if (g_once_init_enter (&regex_initialized))

diff --git a/src/ostree/ot-builtin-pull-local.c b/src/ostree/ot-builtin-pull-local.c
index 92d53afb2ae6223e0a71f1121c7d0ed4ba2d55d8..5be9661ce76e7c3b2c0d5348f93008402466a82d 100644 (file)
--- a/src/ostree/ot-builtin-pull-local.c
+++ b/src/ostree/ot-builtin-pull-local.c
@@ -164,6 +164,14 @@ ostree_builtin_pull_local (int argc, char **argv, OstreeCommandInvocation *invoc
         {
           const char *ref = argv[i];
 
+          /* Validate ref name is valid UTF-8 */
+          if (!g_utf8_validate (ref, -1, NULL))
+            {
+              g_set_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT,
+                           "Invalid ref name: not valid UTF-8");
+              goto out;
+            }
+
           g_ptr_array_add (refs_to_fetch, (char *)ref);
         }
       g_ptr_array_add (refs_to_fetch, NULL);

diff --git a/src/ostree/ot-builtin-pull.c b/src/ostree/ot-builtin-pull.c
index 477a505a4fce7ebeaeab737d3c948a918068d9e3..5133e9b67f99e469e529a797e7bfa8fbb45323eb 100644 (file)
--- a/src/ostree/ot-builtin-pull.c
+++ b/src/ostree/ot-builtin-pull.c
@@ -262,10 +262,19 @@ ostree_builtin_pull (int argc, char **argv, OstreeCommandInvocation *invocation,
                 {
                   guint j;
                   const char *override_commit_id = at + 1;
+                  g_autofree char *ref_name = g_strndup (argv[i], at - argv[i]);
 
                   if (!ostree_validate_checksum_string (override_commit_id, error))
                     goto out;
 
+                  /* Validate ref name is valid UTF-8 */
+                  if (!g_utf8_validate (ref_name, -1, NULL))
+                    {
+                      g_set_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT,
+                                   "Invalid ref name: not valid UTF-8");
+                      goto out;
+                    }
+
                   if (!override_commit_ids)
                     {
                       override_commit_ids = g_ptr_array_new_with_free_func (g_free);
@@ -276,10 +285,18 @@ ostree_builtin_pull (int argc, char **argv, OstreeCommandInvocation *invocation,
                     }
 
                   g_ptr_array_add (override_commit_ids, g_strdup (override_commit_id));
-                  g_ptr_array_add (refs_to_fetch, g_strndup (argv[i], at - argv[i]));
+                  g_ptr_array_add (refs_to_fetch, g_steal_pointer (&ref_name));
                 }
               else
                 {
+                  /* Validate ref name is valid UTF-8 */
+                  if (!g_utf8_validate (argv[i], -1, NULL))
+                    {
+                      g_set_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT,
+                                   "Invalid ref name: not valid UTF-8");
+                      goto out;
+                    }
+
                   g_ptr_array_add (refs_to_fetch, g_strdup (argv[i]));
                   if (override_commit_ids)
                     g_ptr_array_add (override_commit_ids, g_strdup (""));

diff --git a/tests/test-validate-utf8.c b/tests/test-validate-utf8.c
new file mode 100644 (file)
index 0000000..dc561a2
--- /dev/null
+++ b/tests/test-validate-utf8.c
@@ -0,0 +1,102 @@
+/*
+ * SPDX-License-Identifier: LGPL-2.0+
+ *
+ * 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, see <https://www.gnu.org/licenses/>.
+ */
+
+#include "config.h"
+
+#include <gio/gio.h>
+#include <glib.h>
+#include <string.h>
+
+#include "libglnx.h"
+#include "ostree.h"
+
+/* Test that valid UTF-8 refs are accepted */
+static void
+test_valid_utf8_refs (void)
+{
+  g_autoptr (GError) error = NULL;
+
+  /* Valid ASCII ref */
+  g_assert_true (ostree_validate_rev ("my-branch", &error));
+  g_assert_no_error (error);
+
+  /* Valid ref with numbers */
+  g_assert_true (ostree_validate_rev ("branch-123", &error));
+  g_assert_no_error (error);
+
+  /* Valid ref with dots and underscores */
+  g_assert_true (ostree_validate_rev ("my.branch_name", &error));
+  g_assert_no_error (error);
+
+  /* Valid ref with remote */
+  g_assert_true (ostree_validate_rev ("remote:branch", &error));
+  g_assert_no_error (error);
+
+  /* Valid ref with slashes */
+  g_assert_true (ostree_validate_rev ("path/to/branch", &error));
+  g_assert_no_error (error);
+}
+
+/* Test that invalid UTF-8 refs are rejected */
+static void
+test_invalid_utf8_refs (void)
+{
+  g_autoptr (GError) error = NULL;
+
+  /* Invalid UTF-8: 0xFF is never valid in UTF-8 */
+  const char invalid_ff[] = { 'b', 'r', 'a', 'n', 'c', 'h', '\xff', '\0' };
+  g_assert_false (ostree_validate_rev (invalid_ff, &error));
+  g_assert_nonnull (error);
+  g_clear_error (&error);
+
+  /* Invalid UTF-8: 0xFE is never valid in UTF-8 */
+  const char invalid_fe[] = { 'b', 'r', 'a', 'n', 'c', 'h', '\xfe', '\0' };
+  g_assert_false (ostree_validate_rev (invalid_fe, &error));
+  g_assert_nonnull (error);
+  g_clear_error (&error);
+
+  /* Invalid UTF-8: 0x80 without leading byte */
+  const char invalid_80[] = { 'b', 'r', 'a', 'n', 'c', 'h', '\x80', '\0' };
+  g_assert_false (ostree_validate_rev (invalid_80, &error));
+  g_assert_nonnull (error);
+  g_clear_error (&error);
+
+  /* Invalid UTF-8: truncated multi-byte sequence */
+  const char invalid_trunc[] = { 'b', 'r', 'a', 'n', 'c', 'h', '\xc3', '\0' };
+  g_assert_false (ostree_validate_rev (invalid_trunc, &error));
+  g_assert_nonnull (error);
+  g_clear_error (&error);
+
+  /* Invalid UTF-8: 0x333 as mentioned in issue #2959 */
+  const char invalid_333[] = { 's', 'o', 'm', 'e', '-', 'b', 'r', 'a', 'n', 'c',
+                               'h', '-', 'n', 'a', 'm', 'e', '-', 'w', 'i', 't',
+                               'h', '-', 'i', 'n', 'v', 'a', 'l', 'i', 'd', '-',
+                               'u', 't', 'f', '-', 's', 'y', 'm', 'b', 'o', 'l',
+                               '-', '\xdb', '\0' };
+  g_assert_false (ostree_validate_rev (invalid_333, &error));
+  g_assert_nonnull (error);
+  g_clear_error (&error);
+}
+
+int
+main (int argc, char **argv)
+{
+  g_test_init (&argc, &argv, NULL);
+  g_test_add_func ("/validate/valid-utf8-refs", test_valid_utf8_refs);
+  g_test_add_func ("/validate/invalid-utf8-refs", test_invalid_utf8_refs);
+  return g_test_run ();
+}