Run argv click actions through a login shell as positional params
Quickshell.execDetached(argv) ran the click target with only the shell process's stripped environment, so GUI actions like the screenshot editor (tensaku-edit) — resolved on the login-shell PATH the old `bash -lc` string exec provided — stopped launching on click. Run the argv through `bash -lc 'exec "$@"'` instead: the script text is a constant and the arguments are passed as positional parameters, which bash expands without re-tokenizing or re-evaluating, so injection safety is intact while PATH and session env match the old behavior exactly.
This commit is contained in:
+10
-6
@@ -94,12 +94,16 @@ an action and just expect click-to-jump.
|
|||||||
### Click commands must be argv, not shell strings
|
### Click commands must be argv, not shell strings
|
||||||
|
|
||||||
Prefer `--exec-arg` for every click command. Each `--exec-arg` contributes one
|
Prefer `--exec-arg` for every click command. Each `--exec-arg` contributes one
|
||||||
literal argument; the shell runs the resulting vector with
|
literal argument; the shell runs the resulting vector through
|
||||||
`Quickshell.execDetached(argv)` and **no shell**, so a value carrying data an
|
`Util.execArgv`, which invokes `bash -lc 'exec "$@"'` with the arguments as
|
||||||
attacker controls — a downloaded video's title, a received filename, a crashed
|
**positional parameters** — never interpolated into the script text. bash
|
||||||
process's name — is only ever a single argument and can never be reparsed as a
|
expands `"$@"` without re-tokenizing or re-evaluating it, so a value carrying
|
||||||
command. This is the parameterized form: pass untrusted data as its own
|
data an attacker controls — a downloaded video's title, a received filename, a
|
||||||
`--exec-arg` rather than quoting it into a string.
|
crashed process's name — is only ever a single argument and can never be
|
||||||
|
reparsed as a command. The login shell keeps the PATH and session environment
|
||||||
|
that GUI click targets (the screenshot editor, mpv, xdg-open) expect. This is
|
||||||
|
the parameterized form: pass untrusted data as its own `--exec-arg` rather than
|
||||||
|
quoting it into a string.
|
||||||
|
|
||||||
`--exec` is the legacy free-form variant, run through `bash -lc`. It is safe
|
`--exec` is the legacy free-form variant, run through `bash -lc`. It is safe
|
||||||
only when the caller shell-quoted every interpolated value perfectly — the same
|
only when the caller shell-quoted every interpolated value perfectly — the same
|
||||||
|
|||||||
+10
-5
@@ -54,12 +54,17 @@ QtObject {
|
|||||||
Quickshell.execDetached(["bash", "-lc", command])
|
Quickshell.execDetached(["bash", "-lc", command])
|
||||||
}
|
}
|
||||||
|
|
||||||
// Run an argv vector directly, without a shell. Nothing in the array is
|
// Run an argv vector safely: the script text is the constant `exec "$@"`, so
|
||||||
// reparsed, so an argument carrying attacker-controlled data (a filename, a
|
// the arguments only ever land in bash's positional parameters, which it
|
||||||
// title) can never turn into a command. Prefer this over execDetached for any
|
// expands without re-tokenizing or re-evaluating — a value carrying
|
||||||
// command assembled from untrusted input.
|
// attacker-controlled data ($(id), a filename, a title) stays one literal
|
||||||
|
// argument and can never turn into a command. The login shell (-l) is what
|
||||||
|
// makes this a drop-in for execDetached: click actions launch GUI apps
|
||||||
|
// (tensaku, mpv, xdg-open) that need the same PATH and session environment the
|
||||||
|
// login shell set up. Prefer this over execDetached for any command assembled
|
||||||
|
// from untrusted input.
|
||||||
function execArgv(argv) {
|
function execArgv(argv) {
|
||||||
Quickshell.execDetached(argv)
|
Quickshell.execDetached(["bash", "-lc", 'exec "$@"', "bash"].concat(argv))
|
||||||
}
|
}
|
||||||
|
|
||||||
function isPlainObject(value) {
|
function isPlainObject(value) {
|
||||||
|
|||||||
@@ -75,7 +75,8 @@ function execFromHints(hints) {
|
|||||||
|
|
||||||
// The click action as an argv vector, sent by omarchy-notification-send
|
// The click action as an argv vector, sent by omarchy-notification-send
|
||||||
// --exec-arg and carried as a JSON array string in the omarchy-exec-argv hint.
|
// --exec-arg and carried as a JSON array string in the omarchy-exec-argv hint.
|
||||||
// The shell runs it with Quickshell.execDetached (no shell), so a value that an
|
// The shell runs it via Util.execArgv, which passes the arguments as bash
|
||||||
|
// positional parameters rather than interpolating them, so a value that an
|
||||||
// attacker controls — a video title, a filename, a URL — is only ever one
|
// attacker controls — a video title, a filename, a URL — is only ever one
|
||||||
// argument and can never be reparsed as a command. This is the parameterized
|
// argument and can never be reparsed as a command. This is the parameterized
|
||||||
// form: the "prepared statement" to execFromHints's string concatenation.
|
// form: the "prepared statement" to execFromHints's string concatenation.
|
||||||
|
|||||||
@@ -361,10 +361,11 @@ Item {
|
|||||||
if (index < 0 || index >= popupModel.count) return
|
if (index < 0 || index >= popupModel.count) return
|
||||||
var entry = popupModel.get(index)
|
var entry = popupModel.get(index)
|
||||||
|
|
||||||
// Preferred path: an argv vector run without a shell, so data an attacker
|
// Preferred path: an argv vector whose arguments are passed as bash
|
||||||
// controls (a video title, a filename) is only ever an argument and can
|
// positional parameters (never interpolated into a command), so data an
|
||||||
// never be reparsed as a command. Detached so it outlives the shell, which
|
// attacker controls (a video title, a filename) is only ever an argument and
|
||||||
// the installer toasts depend on: they restart the shell as their first act.
|
// can never be reparsed as a command. Detached so it outlives the shell,
|
||||||
|
// which the installer toasts depend on: they restart it as their first act.
|
||||||
var argv = NotificationLogic.parseExecArgv(entry ? entry.execArgv : "")
|
var argv = NotificationLogic.parseExecArgv(entry ? entry.execArgv : "")
|
||||||
if (argv) {
|
if (argv) {
|
||||||
Util.execArgv(argv)
|
Util.execArgv(argv)
|
||||||
|
|||||||
Reference in New Issue
Block a user