Hi, On Tue, Jul 20, 2021 at 08:03:17PM +0200, Salvatore Bonaccorso wrote: > [x] attach debdiff against the package in testing
... or rather not ... attached. Regards, Salvatore
diff -Nru systemd-247.3/debian/changelog systemd-247.3/debian/changelog --- systemd-247.3/debian/changelog 2021-04-12 20:21:24.000000000 +0200 +++ systemd-247.3/debian/changelog 2021-07-13 19:29:24.000000000 +0200 @@ -1,3 +1,13 @@ +systemd (247.3-6) unstable; urgency=high + + * Non-maintainer upload (acked by maintainers) + * unit-name: generate a clear error code when converting an overly long fs + path to a unit name + * basic/unit-name: do not use strdupa() on a path (CVE-2021-33910) + * basic/unit-name: adjust comments + + -- Salvatore Bonaccorso <car...@debian.org> Tue, 13 Jul 2021 19:29:24 +0200 + systemd (247.3-5) unstable; urgency=medium * udev-udeb: setup /dev/fd, /dev/std{in,out,err} symlinks. diff -Nru systemd-247.3/debian/patches/basic-unit-name-adjust-comments.patch systemd-247.3/debian/patches/basic-unit-name-adjust-comments.patch --- systemd-247.3/debian/patches/basic-unit-name-adjust-comments.patch 1970-01-01 01:00:00.000000000 +0100 +++ systemd-247.3/debian/patches/basic-unit-name-adjust-comments.patch 2021-07-13 19:29:07.000000000 +0200 @@ -0,0 +1,38 @@ +From cbcea9f517bfe79b019fcec5c364952ea33d24f2 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbys...@in.waw.pl> +Date: Wed, 23 Jun 2021 11:52:56 +0200 +Subject: basic/unit-name: adjust comments +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +We already checked for "too long" right aboveā¦ +--- + src/basic/unit-name.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/src/basic/unit-name.c b/src/basic/unit-name.c +index a22763443fdd..1deead74588b 100644 +--- a/src/basic/unit-name.c ++++ b/src/basic/unit-name.c +@@ -528,7 +528,7 @@ int unit_name_from_path(const char *path, const char *suffix, char **ret) { + if (strlen(s) >= UNIT_NAME_MAX) /* Return a slightly more descriptive error for this specific condition */ + return -ENAMETOOLONG; + +- /* Refuse this if this got too long or for some other reason didn't result in a valid name */ ++ /* Refuse if this for some other reason didn't result in a valid name */ + if (!unit_name_is_valid(s, UNIT_NAME_PLAIN)) + return -EINVAL; + +@@ -562,7 +562,7 @@ int unit_name_from_path_instance(const char *prefix, const char *path, const cha + if (strlen(s) >= UNIT_NAME_MAX) /* Return a slightly more descriptive error for this specific condition */ + return -ENAMETOOLONG; + +- /* Refuse this if this got too long or for some other reason didn't result in a valid name */ ++ /* Refuse if this for some other reason didn't result in a valid name */ + if (!unit_name_is_valid(s, UNIT_NAME_INSTANCE)) + return -EINVAL; + +-- +2.32.0 + diff -Nru systemd-247.3/debian/patches/basic-unit-name-do-not-use-strdupa-on-a-path.patch systemd-247.3/debian/patches/basic-unit-name-do-not-use-strdupa-on-a-path.patch --- systemd-247.3/debian/patches/basic-unit-name-do-not-use-strdupa-on-a-path.patch 1970-01-01 01:00:00.000000000 +0100 +++ systemd-247.3/debian/patches/basic-unit-name-do-not-use-strdupa-on-a-path.patch 2021-07-13 19:29:07.000000000 +0200 @@ -0,0 +1,64 @@ +From bae2f0d1109a8c75a7fb89ae6b8d1b6ef8dfab16 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbys...@in.waw.pl> +Date: Wed, 23 Jun 2021 11:46:41 +0200 +Subject: basic/unit-name: do not use strdupa() on a path + +The path may have unbounded length, for example through a fuse mount. + +CVE-2021-33910: attacked controlled alloca() leads to crash in systemd and +ultimately a kernel panic. Systemd parses the content of /proc/self/mountinfo +and each mountpoint is passed to mount_setup_unit(), which calls +unit_name_path_escape() underneath. A local attacker who is able to mount a +filesystem with a very long path can crash systemd and the whole system. + +https://bugzilla.redhat.com/show_bug.cgi?id=1970887 + +The resulting string length is bounded by UNIT_NAME_MAX, which is 256. But we +can't easily check the length after simplification before doing the +simplification, which in turns uses a copy of the string we can write to. +So we can't reject paths that are too long before doing the duplication. +Hence the most obvious solution is to switch back to strdup(), as before +7410616cd9dbbec97cf98d75324da5cda2b2f7a2. +--- + src/basic/unit-name.c | 13 +++++-------- + 1 file changed, 5 insertions(+), 8 deletions(-) + +diff --git a/src/basic/unit-name.c b/src/basic/unit-name.c +index 284a77348316..a22763443fdd 100644 +--- a/src/basic/unit-name.c ++++ b/src/basic/unit-name.c +@@ -378,12 +378,13 @@ int unit_name_unescape(const char *f, char **ret) { + } + + int unit_name_path_escape(const char *f, char **ret) { +- char *p, *s; ++ _cleanup_free_ char *p = NULL; ++ char *s; + + assert(f); + assert(ret); + +- p = strdupa(f); ++ p = strdup(f); + if (!p) + return -ENOMEM; + +@@ -395,13 +396,9 @@ int unit_name_path_escape(const char *f, char **ret) { + if (!path_is_normalized(p)) + return -EINVAL; + +- /* Truncate trailing slashes */ ++ /* Truncate trailing slashes and skip leading slashes */ + delete_trailing_chars(p, "/"); +- +- /* Truncate leading slashes */ +- p = skip_leading_chars(p, "/"); +- +- s = unit_name_escape(p); ++ s = unit_name_escape(skip_leading_chars(p, "/")); + } + if (!s) + return -ENOMEM; +-- +2.32.0 + diff -Nru systemd-247.3/debian/patches/series systemd-247.3/debian/patches/series --- systemd-247.3/debian/patches/series 2021-04-12 20:21:24.000000000 +0200 +++ systemd-247.3/debian/patches/series 2021-07-13 19:29:07.000000000 +0200 @@ -7,6 +7,9 @@ pkg-config-make-prefix-overridable-again.patch LoadCredentials-do-not-assert-on-invalid-syntax.patch network-Delay-addition-of-IPv6-Proxy-NDP-addresses.patch +unit-name-generate-a-clear-error-code-when-convertin.patch +basic-unit-name-do-not-use-strdupa-on-a-path.patch +basic-unit-name-adjust-comments.patch debian/Use-Debian-specific-config-files.patch debian/Bring-tmpfiles.d-tmp.conf-in-line-with-Debian-defaul.patch debian/Make-run-lock-tmpfs-an-API-fs.patch diff -Nru systemd-247.3/debian/patches/unit-name-generate-a-clear-error-code-when-convertin.patch systemd-247.3/debian/patches/unit-name-generate-a-clear-error-code-when-convertin.patch --- systemd-247.3/debian/patches/unit-name-generate-a-clear-error-code-when-convertin.patch 1970-01-01 01:00:00.000000000 +0100 +++ systemd-247.3/debian/patches/unit-name-generate-a-clear-error-code-when-convertin.patch 2021-07-13 19:29:07.000000000 +0200 @@ -0,0 +1,55 @@ +From: Lennart Poettering <lenn...@poettering.net> +Date: Tue, 1 Jun 2021 19:43:55 +0200 +Subject: unit-name: generate a clear error code when converting an overly long + fs path to a unit name +Origin: https://github.com/systemd/systemd/commit/9d5acfab20c5f1177d877d0bec18063c0a6c5929 + +[Salvatore Bonaccorso: Backport to 247.3 for context changes in +src/test/test-unit-name.c] +--- + src/basic/unit-name.c | 6 ++++++ + src/test/test-unit-name.c | 4 ++-- + 2 files changed, 8 insertions(+), 2 deletions(-) + +--- a/src/basic/unit-name.c ++++ b/src/basic/unit-name.c +@@ -528,6 +528,9 @@ int unit_name_from_path(const char *path + if (!s) + return -ENOMEM; + ++ if (strlen(s) >= UNIT_NAME_MAX) /* Return a slightly more descriptive error for this specific condition */ ++ return -ENAMETOOLONG; ++ + /* Refuse this if this got too long or for some other reason didn't result in a valid name */ + if (!unit_name_is_valid(s, UNIT_NAME_PLAIN)) + return -EINVAL; +@@ -559,6 +562,9 @@ int unit_name_from_path_instance(const c + if (!s) + return -ENOMEM; + ++ if (strlen(s) >= UNIT_NAME_MAX) /* Return a slightly more descriptive error for this specific condition */ ++ return -ENAMETOOLONG; ++ + /* Refuse this if this got too long or for some other reason didn't result in a valid name */ + if (!unit_name_is_valid(s, UNIT_NAME_INSTANCE)) + return -EINVAL; +--- a/src/test/test-unit-name.c ++++ b/src/test/test-unit-name.c +@@ -130,7 +130,7 @@ static void test_unit_name_from_path(voi + test_unit_name_from_path_one("///", ".mount", "-.mount", 0); + test_unit_name_from_path_one("/foo/../bar", ".mount", NULL, -EINVAL); + test_unit_name_from_path_one("/foo/./bar", ".mount", NULL, -EINVAL); +- test_unit_name_from_path_one("/waldoaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", ".mount", NULL, -EINVAL); ++ test_unit_name_from_path_one("/waldoaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", ".mount", NULL, -ENAMETOOLONG); + } + + static void test_unit_name_from_path_instance_one(const char *pattern, const char *path, const char *suffix, const char *expected, int ret) { +@@ -160,7 +160,7 @@ static void test_unit_name_from_path_ins + test_unit_name_from_path_instance_one("waldo", "..", ".mount", NULL, -EINVAL); + test_unit_name_from_path_instance_one("waldo", "/foo", ".waldi", NULL, -EINVAL); + test_unit_name_from_path_instance_one("wa--ldo", "/--", ".mount", "wa--ldo@\\x2d\\x2d.mount", 0); +- test_unit_name_from_path_instance_one("waldoaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "/waldo", ".mount", NULL, -EINVAL); ++ test_unit_name_from_path_instance_one("waldoaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "/waldo", ".mount", NULL, -ENAMETOOLONG); + } + + static void test_unit_name_to_path_one(const char *unit, const char *path, int ret) {