Assorted remote/shared/resolved hardening fixes flagged by kres (#42978)

src/journal-remote/journal-compression-util.c

@@ -41,7 +41,7 @@ static int compression_config_put(OrderedHashmap **configs, Compression c, int l
                                 free(ordered_hashmap_remove(*configs, INT_TO_PTR(cc->algorithm)));
         } else
                 /* otherwise, drop 'none' if stored. */
-                free(ordered_hashmap_get(*configs, INT_TO_PTR(COMPRESSION_NONE)));
+                free(ordered_hashmap_remove(*configs, INT_TO_PTR(COMPRESSION_NONE)));
 
         return 1;
 }

src/journal-remote/journal-remote-main.c

@@ -532,8 +532,9 @@ static int setup_microhttpd_server(RemoteServer *s,
         d = new(MHDDaemonWrapper, 1);
         if (!d)
                 return log_oom();
-
-        d->fd = (uint64_t) fd;
+        *d = (MHDDaemonWrapper) {
+                .fd = (uint64_t) fd,
+        };
 
         d->daemon = sym_MHD_start_daemon(
                         flags,

src/journal-remote/meson.build

@@ -67,7 +67,7 @@ executables += [
         test_template + {
                 'sources' : files('test-journal-header-util.c'),
                 'conditions' : ['ENABLE_REMOTE', 'HAVE_LIBCURL'],
-                'import' : ['systemd-journal-upload'],
+                'import' : ['systemd-journal-remote', 'systemd-journal-upload'],
         },
         fuzz_template + {
                 'sources' : files('fuzz-journal-remote.c'),

src/journal-remote/test-journal-header-util.c

@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 
 #include "hashmap.h"
+#include "journal-compression-util.h"
 #include "journal-header-util.h"
 #include "tests.h"
 
@@ -18,4 +19,30 @@ TEST(header_put) {
         ASSERT_ERROR(header_put(&headers, "", "Value"), EINVAL);
 }
 
+TEST(compression_none_then_algorithm) {
+        _cleanup_ordered_hashmap_free_ OrderedHashmap *configs = NULL;
+        Compression compression = COMPRESSION_NONE;
+
+        for (Compression c = 1; c < _COMPRESSION_MAX; c++)
+                if (compression_supported(c)) {
+                        compression = c;
+                        break;
+                }
+
+        if (compression == COMPRESSION_NONE)
+                return (void) log_tests_skipped("No compression algorithm supported");
+
+        ASSERT_OK_POSITIVE(config_parse_compression(
+                        "test", "test.conf", 1, "Upload", 1, "Compression", false, "no", &configs, NULL));
+        ASSERT_TRUE(ordered_hashmap_contains(configs, INT_TO_PTR(COMPRESSION_NONE)));
+
+        ASSERT_OK_POSITIVE(config_parse_compression(
+                        "test", "test.conf", 2, "Upload", 1, "Compression", false,
+                        compression_to_string(compression), &configs, NULL));
+        ASSERT_FALSE(ordered_hashmap_contains(configs, INT_TO_PTR(COMPRESSION_NONE)));
+        ASSERT_NOT_NULL(ordered_hashmap_get(configs, INT_TO_PTR(compression)));
+
+        ASSERT_OK(compression_configs_mangle(&configs));
+}
+
 DEFINE_TEST_MAIN(LOG_DEBUG);

src/locale/test-localed-util.c

@@ -79,6 +79,11 @@ TEST(vconsole_convert_to_x11) {
         ASSERT_OK(vconsole_convert_to_x11(&vc, x11_context_verify, &xc));
         ASSERT_TRUE(x11_context_isempty(&xc));
 
+        log_info("/* test leading-dash keymap */");
+        ASSERT_OK(free_and_strdup(&vc.keymap, "-foo"));
+        ASSERT_ERROR(vconsole_convert_to_x11(&vc, x11_context_verify, &xc), EINVAL);
+        ASSERT_TRUE(x11_context_isempty(&xc));
+
         log_info("/* test without variant, new mapping (es:) */");
         ASSERT_OK(free_and_strdup(&vc.keymap, "es"));
         ASSERT_OK(vconsole_convert_to_x11(&vc, x11_context_verify, &xc));
@@ -148,6 +153,11 @@ TEST(x11_convert_to_vconsole) {
         ASSERT_OK(x11_convert_to_vconsole(&xc, &vc));
         ASSERT_TRUE(vc_context_isempty(&vc));
 
+        log_info("/* test leading-comma layout */");
+        ASSERT_OK(free_and_strdup(&xc.layout, ",us"));
+        ASSERT_ERROR(x11_convert_to_vconsole(&xc, &vc), EINVAL);
+        ASSERT_TRUE(vc_context_isempty(&vc));
+
         log_info("/* test without variant, new mapping (es:) */");
         ASSERT_OK(free_and_strdup(&xc.layout, "es"));
         ASSERT_OK(x11_convert_to_vconsole(&xc, &vc));

src/resolve/resolved-dns-query.c

@@ -1339,6 +1339,11 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname)
         if (r == 0 && k == 0) /* No actual cname happened? */
                 return -ELOOP;
 
+        if (r == 0)
+                nq_idna = dns_question_ref(q->question_idna);
+        if (k == 0)
+                nq_utf8 = dns_question_ref(q->question_utf8);
+
         if (q->answer_protocol == DNS_PROTOCOL_DNS)
                 /* Don't permit CNAME redirects from unicast DNS to LLMNR or MulticastDNS, so that global resources
                  * cannot invade the local namespace. The opposite way we permit: local names may redirect to global

src/resolve/test-dns-query.c

@@ -522,7 +522,7 @@ TEST(dns_query_process_cname_one_success_match_dname_utf8_different) {
         ASSERT_EQ(query->n_cname_redirects, 1u);
 
         ASSERT_EQ(dns_question_size(query->collected_questions), 2u);
-        ASSERT_NULL(query->question_utf8);
+        ASSERT_EQ(dns_question_size(query->question_utf8), 1u);
         ASSERT_EQ(dns_question_size(query->question_idna), 1u);
 
         key = dns_resource_key_new(DNS_CLASS_IN, DNS_TYPE_A, "www.\xF0\x9F\x98\xB1.com");
@@ -535,6 +535,11 @@ TEST(dns_query_process_cname_one_success_match_dname_utf8_different) {
         ASSERT_TRUE(dns_question_contains_key(query->collected_questions, key));
         dns_resource_key_unref(key);
 
+        key = dns_resource_key_new(DNS_CLASS_IN, DNS_TYPE_A, "www.\xF0\x9F\x98\xB1.com");
+        ASSERT_NOT_NULL(key);
+        ASSERT_TRUE(dns_question_contains_key(query->question_utf8, key));
+        dns_resource_key_unref(key);
+
         key = dns_resource_key_new(DNS_CLASS_IN, DNS_TYPE_A, "www.v2.xn--tl8h.com");
         ASSERT_NOT_NULL(key);
         ASSERT_TRUE(dns_question_contains_key(query->question_idna, key));

src/shared/rm-rf.c

@@ -443,7 +443,20 @@ int rm_rf_at(int dir_fd, const char *path, RemoveFlags flags) {
 
         /* We refuse to clean the root file system with this call. This is extra paranoia to never cause a
          * really seriously broken system. */
-        if (path_is_root_at(dir_fd, path) > 0)
+        r = path_is_root_at(dir_fd, path);
+        if (r == -ENOENT) {
+                struct stat st;
+
+                if (fstatat(dir_fd, path, &st, AT_SYMLINK_NOFOLLOW) < 0)
+                        r = -errno;
+                else
+                        r = 0; /* The entry exists, but is most likely a dangling symlink. */
+        }
+        if (FLAGS_SET(flags, REMOVE_MISSING_OK) && r == -ENOENT)
+                return 0;
+        if (r < 0)
+                return log_error_errno(r, "Failed to determine whether '%s' is the root file system: %m", path);
+        if (r > 0)
                 return log_error_errno(SYNTHETIC_ERRNO(EPERM),
                                        "Attempted to remove entire root file system, and we can't allow that.");
 

src/shared/shift-uid.c

@@ -295,8 +295,9 @@ static int is_fs_fully_userns_compatible(const struct statfs *sfs) {
                F_TYPE_EQUAL(sfs->f_type, SYSFS_MAGIC);
 }
 
-static int recurse_fd(int fd, const struct stat *st, uid_t shift, bool is_toplevel) {
+static int recurse_fd(int input_fd, const struct stat *st, uid_t shift, bool is_toplevel) {
         _cleanup_closedir_ DIR *d = NULL;
+        _cleanup_close_ int fd = input_fd;
         bool changed = false;
         struct statfs sfs;
         int r;

src/shared/vconsole-util.c

@@ -352,9 +352,11 @@ int find_converted_keymap(const X11Context *xc, char **ret) {
         int r;
 
         assert(xc);
-        assert(!isempty(xc->layout));
         assert(ret);
 
+        if (isempty(xc->layout))
+                return -EINVAL;
+
         if (xc->variant)
                 n = strjoin(xc->layout, "-", xc->variant);
         else

src/sysext/sysext.c

@@ -1808,6 +1808,10 @@ static int unmerge_hierarchy(const Context *c, const char *p, const char *submou
                         l = cunescape_length(escaped_work_dir_in_root, r, 0, &work_dir_in_root);
                         if (l < 0)
                                 return log_error_errno(l, "Failed to unescape work directory path: %m");
+                        if (path_is_absolute(work_dir_in_root) || !path_is_normalized(work_dir_in_root))
+                                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
+                                                       "Invalid work directory path '%s'.", work_dir_in_root);
+
                         work_dir = path_join(c->root, work_dir_in_root);
                         if (!work_dir)
                                 return log_oom();

src/test/test-rm-rf.c

@@ -119,4 +119,20 @@ TEST(rm_rf_chmod) {
         test_rm_rf_chmod_inner();
 }
 
+TEST(rm_rf_dangling_symlink) {
+        _cleanup_(rm_rf_physical_and_freep) char *d = NULL;
+        const char *link;
+
+        ASSERT_OK(mkdtemp_malloc("/tmp/test-rm-rf-dangling.XXXXXXX", &d));
+        link = strjoina(d, "/link");
+
+        ASSERT_OK_ERRNO(symlink("missing", link));
+        ASSERT_OK(rm_rf(link, REMOVE_ROOT|REMOVE_PHYSICAL));
+        ASSERT_ERROR_ERRNO(lstat(link, &(struct stat) {}), ENOENT);
+
+        ASSERT_OK_ERRNO(symlink("missing", link));
+        ASSERT_OK(rm_rf(link, REMOVE_ROOT|REMOVE_PHYSICAL|REMOVE_MISSING_OK));
+        ASSERT_ERROR_ERRNO(lstat(link, &(struct stat) {}), ENOENT);
+}
+
 DEFINE_TEST_MAIN(LOG_DEBUG);

test/units/TEST-50-DISSECT.sysext.sh

@@ -798,6 +798,49 @@ test ! -f "$fake_root$hierarchy/now-is-mutable"
 )
 
 
+( init_trap
+: "Malformed work_dir metadata is rejected without removing unrelated paths"
+[[ -z "$roots_dir" ]] && exit 0
+
+fake_root="$roots_dir/empty-work-dir"
+hierarchy=/opt
+extension_data_dir="$fake_root/var/lib/extensions.mutable$hierarchy"
+work_dir_file="$fake_root$hierarchy/.systemd-sysext/work_dir"
+traversal_target="$roots_dir/work-dir-traversal-target"
+absolute_target="$fake_root/work-dir-absolute-target"
+
+[[ "$FSTYPE" == "fuseblk" ]] && exit 0
+
+prepare_root "$fake_root" "$hierarchy"
+prepare_extension_image "$fake_root" "$hierarchy"
+prepare_read_only_hierarchy "$fake_root" "$hierarchy"
+touch "$fake_root/root-sentinel"
+mkdir "$traversal_target" "$absolute_target"
+touch "$traversal_target/sentinel" "$absolute_target/sentinel"
+prepend_trap "rm -rf ${traversal_target@Q} ${absolute_target@Q}"
+
+run_systemd_sysext "$fake_root" --mutable=yes merge
+prepend_trap "rm -rf ${extension_data_dir@Q}"
+
+# Mutable overlays bind-mount the metadata directory read-only. Unmount that bind first to simulate
+# corrupted on-disk metadata, then try malformed work_dir values.
+umount "$fake_root$hierarchy/.systemd-sysext"
+work_dir=$(<"$work_dir_file")
+
+for invalid_work_dir in "" ../work-dir-traversal-target /work-dir-absolute-target; do
+    printf '%s\n' "$invalid_work_dir" >"$work_dir_file"
+    (! run_systemd_sysext "$fake_root" unmerge)
+    test -f "$fake_root/root-sentinel"
+    test -f "$traversal_target/sentinel"
+    test -f "$absolute_target/sentinel"
+done
+
+# Restore valid metadata and unmerge normally, so the test case leaves no mounted hierarchy behind.
+printf '%s\n' "$work_dir" >"$work_dir_file"
+run_systemd_sysext "$fake_root" unmerge
+)
+
+
 ( init_trap
 : "/var/lib/extensions.mutable/… does not exist, auto-mutability, read-only merged"
 fake_root=${roots_dir:+"$roots_dir/simple-read-only-explicit"}