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) {

Reply via email to