From a3bf0f2f860ff9e73c9062d9f3c400bc672496ac Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Mon, 29 Jun 2026 14:01:08 +0100 Subject: [PATCH 1/3] fido2: reject zero-length HMAC secret The CTAP2 HMAC-SECRET output should never be zero length, so enforce that at both retrieval sites. Follow-up for 1c0c4a43c6118aa4057222789e4b777b61e4bb27 --- src/shared/libfido2-util.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/shared/libfido2-util.c b/src/shared/libfido2-util.c index 4a0d7e6d647..07bea6658ec 100644 --- a/src/shared/libfido2-util.c +++ b/src/shared/libfido2-util.c @@ -622,6 +622,8 @@ static int fido2_use_hmac_hash_specific_token( return log_error_errno(SYNTHETIC_ERRNO(EIO), "Failed to retrieve HMAC secret."); hmac_size = sym_fido_assert_hmac_secret_len(a, 0); + /* Should never be zero, sanity check */ + assert(hmac_size > 0); hmac_copy = memdup(hmac, hmac_size); if (!hmac_copy) @@ -1160,6 +1162,8 @@ int fido2_generate_hmac_hash( return log_error_errno(SYNTHETIC_ERRNO(EIO), "Failed to retrieve HMAC secret."); secret_size = sym_fido_assert_hmac_secret_len(a, 0); + /* Should never be zero, sanity check */ + assert(secret_size > 0); secret_copy = memdup(secret, secret_size); if (!secret_copy) From 1958539136e502bdefac1852e23a667b23bfc120 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Mon, 29 Jun 2026 15:05:51 +0100 Subject: [PATCH 2/3] env-util: ensure NUL termination of the replace_env_argv() output array The output array is allocated with new() and left uninitialized, but a bare unset "$VAR" token expands to nothing and writes no terminator. When such a token leads or is the only word, the returned strv is left without a trailing NULL. Follow-up for f331434d13488425ccd8485327085d15f8f92aea --- src/basic/env-util.c | 2 ++ src/test/test-env-util.c | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/basic/env-util.c b/src/basic/env-util.c index 4f6240e5465..934428e7f52 100644 --- a/src/basic/env-util.c +++ b/src/basic/env-util.c @@ -948,6 +948,8 @@ int replace_env_argv( } k += q; + /* reallocarray() does not zero-initialize the grown tail, add NUL termination */ + n[k] = NULL; continue; } diff --git a/src/test/test-env-util.c b/src/test/test-env-util.c index 281ea7951cb..7c55bcc3d86 100644 --- a/src/test/test-env-util.c +++ b/src/test/test-env-util.c @@ -590,4 +590,28 @@ TEST(strv_env_get_merged) { ASSERT_TRUE(strv_equal(m, expected)); } +TEST(replace_env_argv_unterminated) { + /* A bare unset "$VAR" token expands to nothing and advances neither the write index nor writes a + * terminator, so the output strv must still be NUL-terminated when such a token leads or is the + * only word. */ + + _cleanup_strv_free_ char **a = NULL, **b = NULL, **c = NULL, **d = NULL; + + /* Single bare unset variable: result must be a properly terminated empty strv. */ + ASSERT_OK(replace_env_argv(STRV_MAKE("$THIS_IS_UNSET"), STRV_MAKE("FOO=BAR"), &a, NULL, NULL)); + ASSERT_TRUE(strv_isempty(a)); + + /* Only unset variables. */ + ASSERT_OK(replace_env_argv(STRV_MAKE("$THIS_IS_UNSET", "$ALSO_UNSET"), STRV_MAKE("FOO=BAR"), &b, NULL, NULL)); + ASSERT_TRUE(strv_isempty(b)); + + /* Trailing bare unset variable after an expanded one. */ + ASSERT_OK(replace_env_argv(STRV_MAKE("$FOO", "$THIS_IS_UNSET"), STRV_MAKE("FOO=BAR"), &c, NULL, NULL)); + ASSERT_TRUE(strv_equal(c, STRV_MAKE("BAR"))); + + /* Trailing bare unset variable after a literal word. */ + ASSERT_OK(replace_env_argv(STRV_MAKE("hello", "$THIS_IS_UNSET"), STRV_MAKE("FOO=BAR"), &d, NULL, NULL)); + ASSERT_TRUE(strv_equal(d, STRV_MAKE("hello"))); +} + DEFINE_TEST_MAIN(LOG_DEBUG); From 12bf400b13a3e508a29b577b8746f73798f5af52 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Mon, 29 Jun 2026 18:07:01 +0100 Subject: [PATCH 3/3] ptyfwd: avoid touching forwarder after exit drain on_exit_event() can synchronously drain buffered data through shovel_force(). If that completes the drain, pty_forward_done() runs the hangup handler and may free the forwarder, so do not call pty_forward_done() again afterwards. [ 25.052879] TEST-74-AUX-UTILS.sh[909]: ==909==ERROR: AddressSanitizer: heap-use-after-free on address 0x7ccc8a5e0b41 at pc 0x7efc8cde106e bp 0x7ffd668629b0 sp 0x7ffd668629a8 [ 25.053136] TEST-74-AUX-UTILS.sh[909]: READ of size 1 at 0x7ccc8a5e0b41 thread T0 [ 25.092784] TEST-74-AUX-UTILS.sh[909]: #0 0x7efc8cde106d in pty_forward_done ../src/src/shared/ptyfwd.c:187 [ 25.093920] TEST-74-AUX-UTILS.sh[909]: #1 0x7efc8cdedba1 in on_exit_event ../src/src/shared/ptyfwd.c:904 [ 25.094148] TEST-74-AUX-UTILS.sh[909]: #2 0x7efc8d375eff in source_dispatch ../src/src/libsystemd/sd-event/sd-event.c:4301 [ 25.095074] TEST-74-AUX-UTILS.sh[909]: #3 0x7efc8d378032 in dispatch_exit ../src/src/libsystemd/sd-event/sd-event.c:4431 [ 25.095295] TEST-74-AUX-UTILS.sh[909]: #4 0x7efc8d37e932 in sd_event_dispatch ../src/src/libsystemd/sd-event/sd-event.c:4896 [ 25.095467] TEST-74-AUX-UTILS.sh[909]: #5 0x7efc8d37fc8c in sd_event_run ../src/src/libsystemd/sd-event/sd-event.c:4971 [ 25.095647] TEST-74-AUX-UTILS.sh[909]: #6 0x7efc8d3800ad in sd_event_loop ../src/src/libsystemd/sd-event/sd-event.c:4992 [ 25.097174] TEST-74-AUX-UTILS.sh[909]: #7 0x56049b541aba in start_transient_service ../src/src/run/run.c:2479 [ 25.097403] TEST-74-AUX-UTILS.sh[909]: #8 0x56049b552a65 in run ../src/src/run/run.c:3288 [ 25.097569] TEST-74-AUX-UTILS.sh[909]: #9 0x56049b552cb0 in main ../src/src/run/run.c:3291 [ 25.097780] TEST-74-AUX-UTILS.sh[909]: #10 0x7efc8b882300 in __libc_start_call_main (/lib64/libc.so.6+0x7d300) (BuildId: 830c94f480c13d9b01dc65a1035310882136094a) [ 25.097952] TEST-74-AUX-UTILS.sh[909]: #11 0x7efc8b882417 in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x7d417) (BuildId: 830c94f480c13d9b01dc65a1035310882136094a) [ 25.098139] TEST-74-AUX-UTILS.sh[909]: #12 0x56049b51cf54 in _start (/usr/bin/systemd-run+0x19f54) (BuildId: 0daacdb9f20151f3517312ee99e489a9b8f4989c) [ 25.098316] TEST-74-AUX-UTILS.sh[909]: 0x7ccc8a5e0b41 is located 193 bytes inside of 2384-byte region [0x7ccc8a5e0a80,0x7ccc8a5e13d0) [ 25.099202] TEST-74-AUX-UTILS.sh[909]: freed by thread T0 here: [ 25.099410] TEST-74-AUX-UTILS.sh[909]: #0 0x7efc8e76420f in free.part.0 (/lib64/libasan.so.8+0x16420f) (BuildId: 173395e60f171589489dde2b7a156d0ae380734b) [ 25.099557] TEST-74-AUX-UTILS.sh[909]: #1 0x7efc8cdf14d1 in pty_forward_free ../src/src/shared/ptyfwd.c:1122 [ 25.099691] TEST-74-AUX-UTILS.sh[909]: #2 0x56049b535328 in pty_forward_handler ../src/src/run/run.c:1952 [ 25.100063] TEST-74-AUX-UTILS.sh[909]: #3 0x7efc8cde138c in pty_forward_done ../src/src/shared/ptyfwd.c:196 [ 25.100197] TEST-74-AUX-UTILS.sh[909]: #4 0x7efc8cdec757 in shovel ../src/src/shared/ptyfwd.c:813 [ 25.101144] TEST-74-AUX-UTILS.sh[909]: #5 0x7efc8cdecc1f in shovel_force ../src/src/shared/ptyfwd.c:828 [ 25.102273] TEST-74-AUX-UTILS.sh[909]: #6 0x7efc8cdedb82 in on_exit_event ../src/src/shared/ptyfwd.c:899 [ 25.103564] TEST-74-AUX-UTILS.sh[909]: #7 0x7efc8d375eff in source_dispatch ../src/src/libsystemd/sd-event/sd-event.c:4301 [ 25.103712] TEST-74-AUX-UTILS.sh[909]: #8 0x7efc8d378032 in dispatch_exit ../src/src/libsystemd/sd-event/sd-event.c:4431 [ 25.104081] TEST-74-AUX-UTILS.sh[909]: #9 0x7efc8d37e932 in sd_event_dispatch ../src/src/libsystemd/sd-event/sd-event.c:4896 [ 25.104954] TEST-74-AUX-UTILS.sh[909]: #10 0x7efc8d37fc8c in sd_event_run ../src/src/libsystemd/sd-event/sd-event.c:4971 [ 25.105160] TEST-74-AUX-UTILS.sh[909]: #11 0x7efc8d3800ad in sd_event_loop ../src/src/libsystemd/sd-event/sd-event.c:4992 [ 25.105310] TEST-74-AUX-UTILS.sh[909]: #12 0x56049b541aba in start_transient_service ../src/src/run/run.c:2479 [ 25.105454] TEST-74-AUX-UTILS.sh[909]: #13 0x56049b552a65 in run ../src/src/run/run.c:3288 [ 25.105572] TEST-74-AUX-UTILS.sh[909]: #14 0x56049b552cb0 in main ../src/src/run/run.c:3291 [ 25.106136] TEST-74-AUX-UTILS.sh[909]: #15 0x7efc8b882300 in __libc_start_call_main (/lib64/libc.so.6+0x7d300) (BuildId: 830c94f480c13d9b01dc65a1035310882136094a) [ 25.106263] TEST-74-AUX-UTILS.sh[909]: #16 0x7efc8b882417 in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x7d417) (BuildId: 830c94f480c13d9b01dc65a1035310882136094a) [ 25.106385] TEST-74-AUX-UTILS.sh[909]: #17 0x56049b51cf54 in _start (/usr/bin/systemd-run+0x19f54) (BuildId: 0daacdb9f20151f3517312ee99e489a9b8f4989c) [ 25.106792] TEST-74-AUX-UTILS.sh[909]: previously allocated by thread T0 here: [ 25.106957] TEST-74-AUX-UTILS.sh[909]: #0 0x7efc8e76515f in malloc (/lib64/libasan.so.8+0x16515f) (BuildId: 173395e60f171589489dde2b7a156d0ae380734b) [ 25.108013] TEST-74-AUX-UTILS.sh[909]: #1 0x7efc8cddebed in malloc_multiply ../src/src/basic/alloc-util.h:92 [ 25.108188] TEST-74-AUX-UTILS.sh[909]: #2 0x7efc8cdee47b in pty_forward_new ../src/src/shared/ptyfwd.c:955 [ 25.108324] TEST-74-AUX-UTILS.sh[909]: #3 0x56049b538700 in run_context_setup_ptyfwd ../src/src/run/run.c:2130 [ 25.108472] TEST-74-AUX-UTILS.sh[909]: #4 0x56049b5419f9 in start_transient_service ../src/src/run/run.c:2465 [ 25.109152] TEST-74-AUX-UTILS.sh[909]: #5 0x56049b552a65 in run ../src/src/run/run.c:3288 [ 25.109311] TEST-74-AUX-UTILS.sh[909]: #6 0x56049b552cb0 in main ../src/src/run/run.c:3291 [ 25.109450] TEST-74-AUX-UTILS.sh[909]: #7 0x7efc8b882300 in __libc_start_call_main (/lib64/libc.so.6+0x7d300) (BuildId: 830c94f480c13d9b01dc65a1035310882136094a) [ 25.109847] TEST-74-AUX-UTILS.sh[909]: #8 0x7efc8b882417 in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x7d417) (BuildId: 830c94f480c13d9b01dc65a1035310882136094a) [ 25.110760] TEST-74-AUX-UTILS.sh[909]: #9 0x56049b51cf54 in _start (/usr/bin/systemd-run+0x19f54) (BuildId: 0daacdb9f20151f3517312ee99e489a9b8f4989c) [ 25.110911] TEST-74-AUX-UTILS.sh[909]: SUMMARY: AddressSanitizer: heap-use-after-free ../src/src/shared/ptyfwd.c:187 in pty_forward_done [ 25.111054] TEST-74-AUX-UTILS.sh[909]: Shadow bytes around the buggy address: [ 25.111213] TEST-74-AUX-UTILS.sh[909]: 0x7ccc8a5e0880: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa [ 25.111378] TEST-74-AUX-UTILS.sh[909]: 0x7ccc8a5e0900: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa [ 25.111520] TEST-74-AUX-UTILS.sh[909]: 0x7ccc8a5e0980: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa [ 25.112210] TEST-74-AUX-UTILS.sh[909]: 0x7ccc8a5e0a00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa [ 25.112399] TEST-74-AUX-UTILS.sh[909]: 0x7ccc8a5e0a80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd [ 25.112767] TEST-74-AUX-UTILS.sh[909]: =>0x7ccc8a5e0b00: fd fd fd fd fd fd fd fd[fd]fd fd fd fd fd fd fd [ 25.112901] TEST-74-AUX-UTILS.sh[909]: 0x7ccc8a5e0b80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd [ 25.113789] TEST-74-AUX-UTILS.sh[909]: 0x7ccc8a5e0c00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd [ 25.113906] TEST-74-AUX-UTILS.sh[909]: 0x7ccc8a5e0c80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd [ 25.114046] TEST-74-AUX-UTILS.sh[909]: 0x7ccc8a5e0d00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd [ 25.114159] TEST-74-AUX-UTILS.sh[909]: 0x7ccc8a5e0d80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd [ 25.114278] TEST-74-AUX-UTILS.sh[909]: Shadow byte legend (one shadow byte represents 8 application bytes): [ 25.114400] TEST-74-AUX-UTILS.sh[909]: Addressable: 00 [ 25.115099] TEST-74-AUX-UTILS.sh[909]: Partially addressable: 01 02 03 04 05 06 07 [ 25.115246] TEST-74-AUX-UTILS.sh[909]: Heap left redzone: fa [ 25.115365] TEST-74-AUX-UTILS.sh[909]: Freed heap region: fd [ 25.115483] TEST-74-AUX-UTILS.sh[909]: Stack left redzone: f1 [ 25.115618] TEST-74-AUX-UTILS.sh[909]: Stack mid redzone: f2 [ 25.115882] TEST-74-AUX-UTILS.sh[909]: Stack right redzone: f3 [ 25.116735] TEST-74-AUX-UTILS.sh[909]: Stack after return: f5 [ 25.116857] TEST-74-AUX-UTILS.sh[909]: Stack use after scope: f8 [ 25.116974] TEST-74-AUX-UTILS.sh[909]: Global redzone: f9 [ 25.117108] TEST-74-AUX-UTILS.sh[909]: Global init order: f6 [ 25.117257] TEST-74-AUX-UTILS.sh[909]: Poisoned by user: f7 [ 25.118128] TEST-74-AUX-UTILS.sh[909]: Container overflow: fc [ 25.118288] TEST-74-AUX-UTILS.sh[909]: Array cookie: ac [ 25.118433] TEST-74-AUX-UTILS.sh[909]: Intra object redzone: bb [ 25.118546] TEST-74-AUX-UTILS.sh[909]: ASan internal: fe [ 25.118684] TEST-74-AUX-UTILS.sh[909]: Left alloca redzone: ca [ 25.118792] TEST-74-AUX-UTILS.sh[909]: Right alloca redzone: cb [ 25.119282] TEST-74-AUX-UTILS.sh[909]: Command: systemd-run --quiet --pty -- bash -c echo PTY_FORWARD_READY; exec sleep 60 [ 25.119395] TEST-74-AUX-UTILS.sh[909]: ==909==ABORTING Follow-up for d3218c4c80c99583d3e7a31ff2f509ffb097568e --- src/shared/ptyfwd.c | 15 ++++++--------- test/units/TEST-74-AUX-UTILS.run.sh | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c index fcc1500dc5c..5445da2a1b6 100644 --- a/src/shared/ptyfwd.c +++ b/src/shared/ptyfwd.c @@ -889,19 +889,16 @@ static int on_sigwinch_event(sd_event_source *e, const struct signalfd_siginfo * static int on_exit_event(sd_event_source *e, void *userdata) { PTYForward *f = ASSERT_PTR(userdata); - int r; assert(e); assert(e == f->exit_event_source); - if (!pty_forward_drain(f)) { - /* If not drained, try to drain the buffer. */ - r = shovel_force(f); - if (r < 0) - return r; - } - - return pty_forward_done(f, 0); + /* The event loop is exiting while the forwarder is still active. Force a final synchronous drain of + * whatever is still buffered. shovel() may complete the drain and call pty_forward_done(), which + * can free f via the hangup handler, so we must not touch f afterwards, only propagate the return + * value, exactly like on_master_event() does. */ + f->drain = true; + return shovel_force(f); } static int on_defer_event(sd_event_source *s, void *userdata) { diff --git a/test/units/TEST-74-AUX-UTILS.run.sh b/test/units/TEST-74-AUX-UTILS.run.sh index 2f120638b50..bdb0fd1a7eb 100755 --- a/test/units/TEST-74-AUX-UTILS.run.sh +++ b/test/units/TEST-74-AUX-UTILS.run.sh @@ -314,3 +314,19 @@ fi # Tests whether intermediate disconnects corrupt us (modified testcase from https://github.com/systemd/systemd/issues/27204) assert_rc "37" timeout 300 systemd-run --unit=disconnecttest --wait --pipe --user -M testuser@.host bash -ec 'systemctl --user daemon-reexec; sleep 3; exit 37' + +# Trigger on_exit_event(): SIGINT makes systemd-run leave the event loop +# while PTY forwarding is active. coproc gives us a pollable stdout and exec +# makes $PTY_FWD_PID point at systemd-run +coproc PTY_FWD { exec systemd-run --quiet --pty -- bash -c 'echo PTY_FORWARD_READY; exec sleep 60'; } +PTY_FWD_PID_SAVED="$PTY_FWD_PID" +read -r -t 30 -u "${PTY_FWD[0]}" PTY_FWD_LINE || true +[[ "$PTY_FWD_LINE" == *PTY_FORWARD_READY* ]] +kill -INT "$PTY_FWD_PID_SAVED" +PTY_FWD_RC=0 +wait "$PTY_FWD_PID_SAVED" || PTY_FWD_RC=$? +# ASan reports the UAF as systemd-run exiting non-zero. +if [[ "$PTY_FWD_RC" != 0 ]]; then + echo "systemd-run --pty failed" + exit 1 +fi