From 2df1aa7a92667423c827aa99ff397c247c843cfb Mon Sep 17 00:00:00 2001 From: L4XB Date: Thu, 24 Sep 2026 23:11:54 +0200 Subject: [PATCH] fix(gateguard): check every command su is given, not only the first `su postgres -c "echo ok" -c "psql -c 'drop table users'"` runs the last -c, but only the first was scanned, so the drop went through. Every -c/--command value is now scanned as a command line of its own. --- scripts/hooks/gateguard-fact-force.js | 29 +++++++++++++++--------- tests/hooks/gateguard-fact-force.test.js | 2 ++ 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index d7106c6dd..329b3ae16 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -360,22 +360,28 @@ function quoteAwareSegments(input) { const SHELL_WRAPPERS = new Set(['sh', 'bash', 'zsh', 'dash', 'ksh']); /** - * The command line `su` runs through the target user's shell: the value of - * `-c`/`--command`, or of a short-option cluster ending in `c` (`-lc`). `su` - * passes the arguments after `--` to that shell, which runs a `-c` there too, - * so the scan does not stop at `--`. + * The command lines `su` may run through the target user's shell: the value of + * every `-c`/`--command`, or of a short-option cluster ending in `c` (`-lc`). + * `su` runs only the last one, so each is checked rather than guessing which + * wins. `su` passes the arguments after `--` to that shell, which runs a `-c` + * there too, so the scan does not stop at `--`. * * @param {string[]} tokens dequoted tokens for one segment * @param {number} start index of the `su` token - * @returns {string|null} + * @returns {string[]} */ -function suCommandLine(tokens, start) { +function suCommandLines(tokens, start) { + const commandLines = []; for (let i = start + 1; i < tokens.length; i += 1) { const arg = tokens[i]; - if (arg.startsWith('--command=')) return arg.slice('--command='.length); - if (arg === '--command' || /^-[A-Za-z]*c$/.test(arg)) return i + 1 < tokens.length ? tokens[i + 1] : null; + if (arg.startsWith('--command=')) { + commandLines.push(arg.slice('--command='.length)); + } else if ((arg === '--command' || /^-[A-Za-z]*c$/.test(arg)) && i + 1 < tokens.length) { + commandLines.push(tokens[i + 1]); + i += 1; + } } - return null; + return commandLines; } /** @@ -637,8 +643,9 @@ function isDestructiveQuoteAware(raw, depth = 0) { } } if (base === 'su') { - const commandLine = suCommandLine(tokens, wi); - if (commandLine && isDestructiveQuoteAware(commandLine, depth + 1)) return true; + for (const commandLine of suCommandLines(tokens, wi)) { + if (isDestructiveQuoteAware(commandLine, depth + 1)) return true; + } } } return false; diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index 8dddc21c5..32404f81a 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -1556,6 +1556,8 @@ function runTests() { expectDestructiveDeny('su -lc "psql -c \'drop table users\'" postgres', 'su -lc psql'); expectDestructiveDeny('su --command="psql -c \'drop table users\'" postgres', 'su --command= psql'); expectDestructiveDeny('su postgres -- -c "psql -c \'drop table users\'"', 'su -c after --'); + expectDestructiveDeny('su postgres -c "echo ok" -c "psql -c \'drop table users\'"', 'su runs its last -c'); + expectDestructiveDeny('su -c "psql -c \'drop table users\'" -c "echo ok" postgres', 'su -c with a later harmless -c'); expectDestructiveDeny('taskset -c 0 psql -c "drop table users"', 'taskset -c psql'); }) )