From b2311e29bf46e43121955b5bb5aaaafa32d1f20c Mon Sep 17 00:00:00 2001 From: Elie Gambache Date: Wed, 19 Aug 2026 20:24:19 +0300 Subject: [PATCH] fix(updater): close TOCTOU and downgrade in the root Linux update helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The passwordless self-update helper runs as root via pkexec but takes a caller-supplied package path that is writable by the unprivileged invoking user. It verified the detached signature and then re-opened the same path for `dpkg -i` / `rpm -U` — two separate opens, so the bytes could be swapped between verify and install, giving a local user root code execution. The helper now copies the package and its signature into a root-owned 0700 working dir first, then verifies and installs the copies, so the verified bytes cannot be replaced. The deb path additionally refuses anything that is not a strict version increase (`dpkg --compare-versions ... gt`), blocking a rollback to an older, still-validly-signed, vulnerable release; `rpm -U` already refuses downgrades. The bash is exercised on real distros in packaging; a new unit test locks the security-relevant structure (copy-before-verify, verify-the-copy, refuse non-upgrade) so a refactor cannot silently drop it. Co-Authored-By: Claude Opus 4.8 --- .../application/internal/LinuxUpdateHelper.kt | 45 +++++++++++++++---- .../internal/LinuxUpdateHelperTest.kt | 43 ++++++++++++++++++ 2 files changed, 79 insertions(+), 9 deletions(-) create mode 100644 plugin-build/plugin/src/test/kotlin/dev/nucleusframework/desktop/application/internal/LinuxUpdateHelperTest.kt diff --git a/plugin-build/plugin/src/main/kotlin/dev/nucleusframework/desktop/application/internal/LinuxUpdateHelper.kt b/plugin-build/plugin/src/main/kotlin/dev/nucleusframework/desktop/application/internal/LinuxUpdateHelper.kt index 7277d4b8b..ce9e34acc 100644 --- a/plugin-build/plugin/src/main/kotlin/dev/nucleusframework/desktop/application/internal/LinuxUpdateHelper.kt +++ b/plugin-build/plugin/src/main/kotlin/dev/nucleusframework/desktop/application/internal/LinuxUpdateHelper.kt @@ -32,28 +32,48 @@ internal object LinuxUpdateHelper { const val PUBLIC_KEY_RELATIVE_PATH: String = "resources/nucleus-update.pub.asc" /** - * Self-contained bash helper: verify detached signature against the bundled public key, - * ensure the package upgrades only the app that owns this helper, then `dpkg -i` / `rpm -U`. + * Self-contained bash helper: copy the caller-supplied package and signature into a + * root-owned working directory, verify the detached signature of that copy against the + * bundled public key, ensure the package upgrades only the app that owns this helper and is + * not a downgrade, then `dpkg -i` / `rpm -U` the verified copy. + * + * Security notes: + * - The helper runs as **root** via pkexec, but the caller-supplied path is writable by the + * unprivileged invoking user. Verifying that path and then re-opening it for install would + * be a TOCTOU: the bytes could be swapped between `gpg --verify` and `dpkg -i`. Copying into + * a root-owned dir *before* verifying, then installing the copy, removes that window. + * - `dpkg -i` will install an older, still-validly-signed release (a rollback to a known- + * vulnerable version), so the deb path refuses anything that is not a strict version + * increase. `rpm -U` (without `--oldpackage`) already refuses downgrades. */ val SCRIPT: String = $$""" #!/usr/bin/env bash # Installed by Nucleus as a package-owned file. Verifies a signed update against the - # bundled public key and, if valid and for this same app, installs it. + # bundled public key and, if valid, for this same app and a strict upgrade, installs it. # Invoked via pkexec (see polkit policy installed by afterInstall). set -eu if [ "$#" -lt 1 ]; then echo "usage: nucleus-update-helper " >&2; exit 2; fi - PKG="$1" - [ -f "$PKG" ] || { echo "package not found: $PKG" >&2; exit 2; } + SRC_PKG="$1" + [ -f "$SRC_PKG" ] || { echo "package not found: $SRC_PKG" >&2; exit 2; } + SRC_SIG="$SRC_PKG.asc" + [ -f "$SRC_SIG" ] || { echo "missing signature: $SRC_SIG" >&2; exit 4; } SELF="$(readlink -f "$0")" APPDIR="$(dirname "$SELF")" PUBKEY="$APPDIR/resources/nucleus-update.pub.asc" - SIG="$PKG.asc" [ -f "$PUBKEY" ] || { echo "missing public key: $PUBKEY" >&2; exit 4; } - [ -f "$SIG" ] || { echo "missing signature: $SIG" >&2; exit 4; } - # Verify the detached signature against the bundled key in a throwaway keyring. - KR="$(mktemp -d)"; trap 'rm -rf "$KR"' EXIT; chmod 700 "$KR" + # Copy into a root-owned working dir BEFORE verifying, then verify and install the copies. + # The source path is writable by the unprivileged caller, so verifying it and re-opening it + # for install would let the bytes be swapped in between (TOCTOU); the root-owned copy cannot. + WORK="$(mktemp -d)"; trap 'rm -rf "$WORK"' EXIT; chmod 700 "$WORK" + PKG="$WORK/$(basename "$SRC_PKG")" + SIG="$PKG.asc" + cp -- "$SRC_PKG" "$PKG" + cp -- "$SRC_SIG" "$SIG" + + # Verify the detached signature of the copy against the bundled key in a throwaway keyring. + KR="$WORK/keyring"; mkdir -p "$KR"; chmod 700 "$KR" gpg --homedir "$KR" --batch --quiet --import "$PUBKEY" gpg --homedir "$KR" --batch --verify "$SIG" "$PKG" @@ -64,12 +84,19 @@ internal object LinuxUpdateHelper { OWNER="$(dpkg -S "$SELF" 2>/dev/null | cut -d: -f1 | head -n1)" NEW="$(dpkg-deb -f "$PKG" Package)" [ -n "$OWNER" ] && [ "$NEW" = "$OWNER" ] || { echo "package mismatch: $NEW != $OWNER" >&2; exit 3; } + # Refuse downgrades: dpkg -i does not, so a signed older release could roll the app back. + CUR="$(dpkg-query -W -f='${Version}' "$OWNER" 2>/dev/null || true)" + NEWVER="$(dpkg-deb -f "$PKG" Version)" + if [ -n "$CUR" ] && ! dpkg --compare-versions "$NEWVER" gt "$CUR"; then + echo "refusing non-upgrade: $NEWVER is not newer than installed $CUR" >&2; exit 5 + fi exec dpkg -i "$PKG" ;; *.rpm) OWNER="$(rpm -qf --qf '%{NAME}' "$SELF" 2>/dev/null || true)" NEW="$(rpm -qp --qf '%{NAME}' "$PKG")" [ -n "$OWNER" ] && [ "$NEW" = "$OWNER" ] || { echo "package mismatch: $NEW != $OWNER" >&2; exit 3; } + # rpm -U (without --oldpackage) already refuses downgrades. exec rpm -U "$PKG" ;; *) echo "unsupported package: $PKG" >&2; exit 2 ;; diff --git a/plugin-build/plugin/src/test/kotlin/dev/nucleusframework/desktop/application/internal/LinuxUpdateHelperTest.kt b/plugin-build/plugin/src/test/kotlin/dev/nucleusframework/desktop/application/internal/LinuxUpdateHelperTest.kt new file mode 100644 index 000000000..d9032ec9c --- /dev/null +++ b/plugin-build/plugin/src/test/kotlin/dev/nucleusframework/desktop/application/internal/LinuxUpdateHelperTest.kt @@ -0,0 +1,43 @@ +package dev.nucleusframework.desktop.application.internal + +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Structural guards on the root-run update helper script. The helper is invoked as root via + * pkexec, so these invariants (verify a root-owned copy, refuse downgrades) protect against a + * local privilege escalation and a signed-package rollback. The bash itself is exercised on real + * distros in packaging tests; here we lock the security-relevant shape so a refactor cannot drop it. + */ +class LinuxUpdateHelperTest { + private val script = LinuxUpdateHelper.SCRIPT + + @Test + fun `package and signature are copied into a root-owned work dir before verification`() { + assertTrue("creates a private work dir", script.contains("mktemp -d")) + assertTrue("copies the package", script.contains("""cp -- "${'$'}SRC_PKG" "${'$'}PKG"""")) + assertTrue("copies the signature", script.contains("""cp -- "${'$'}SRC_SIG" "${'$'}SIG"""")) + + val copyIndex = script.indexOf("""cp -- "${'$'}SRC_PKG"""") + val verifyIndex = script.indexOf("--verify") + assertTrue("copy must run before verify (no TOCTOU window)", copyIndex in 0 until verifyIndex) + } + + @Test + fun `verification runs against the copied package, not the caller-supplied path`() { + assertTrue("PKG is inside the work dir", script.contains("""PKG="${'$'}WORK/""")) + assertTrue( + "gpg verifies the copy", + script.contains("""gpg --homedir "${'$'}KR" --batch --verify "${'$'}SIG" "${'$'}PKG""""), + ) + } + + @Test + fun `deb path refuses a non-upgrade to block a signed downgrade`() { + assertTrue( + "uses dpkg --compare-versions gt", + script.contains("""dpkg --compare-versions "${'$'}NEWVER" gt "${'$'}CUR""""), + ) + assertTrue("exits on non-upgrade", script.contains("refusing non-upgrade")) + } +}