mirror of
https://github.com/affaan-m/ECC.git
synced 2026-09-22 17:45:15 +02:00
fix(gateguard): detect destructive SQL passed quoted to SQL clients (#3107)
* fix(gateguard): detect destructive SQL passed quoted to SQL clients Quoted SQL like psql -c "drop table users" was stripped before the DESTRUCTIVE_SQL_DD regex ran, making it unreachable. Check dequoted tokens for known SQL clients (psql, mysql, sqlite3, ...) so real invocations are gated while git commit -m / echo mentions stay allowed. Fixes affaan-m/ECC#3024. * fix(gateguard): unwrap sudo/env flags and ignore SQL string literals Address review: sudo -u/env VAR= wrappers now resolve to the real SQL client; SQL single/double/dollar-quoted literals are stripped before the destructive regex so SELECT 'drop table' stays allowed. Adds deny/allow regression tests for quoted SQL, wrappers, literals, and non-SQL clients. * test(gateguard): cover env PGPASSWORD wrapper variant * fix(gateguard): close wrapper sh-c, env dir, tagged literal gaps Review 5189686662: recurse sh -c from unwrapped wrapper index (sudo/doas/env + sh -c now denied); consume env -C/--chdir separate values; strip tagged dollar-quoted literals ($tag$...\baseline). Tests: 200 passed.
This commit is contained in:
Varun Gore
@@ -359,6 +359,155 @@ function quoteAwareSegments(input) {
|
||||
|
||||
const SHELL_WRAPPERS = new Set(['sh', 'bash', 'zsh', 'dash', 'ksh']);
|
||||
|
||||
/**
|
||||
* SQL clients whose `-c`/`-e`/positional arguments carry SQL statements.
|
||||
* Quoted SQL (e.g. `psql -c "drop table users"`) is invisible to the
|
||||
* quote-stripping SQL regex, so it is re-checked here against dequoted
|
||||
* tokens where quoted content is preserved (issue #3024). Restricted to
|
||||
* known clients so `git commit -m "drop table"` and `echo "drop table"`
|
||||
* stay allowed.
|
||||
*/
|
||||
const SQL_CLIENT_COMMANDS = new Set([
|
||||
'psql',
|
||||
'postgres',
|
||||
'mysql',
|
||||
'mariadb',
|
||||
'sqlite3',
|
||||
'sqlite',
|
||||
'sqlcmd',
|
||||
'isql',
|
||||
'pgcli',
|
||||
'mycli',
|
||||
'duckdb',
|
||||
'bq',
|
||||
]);
|
||||
|
||||
/**
|
||||
* Strip SQL string literals so phrases inside query data do not trigger
|
||||
* the destructive detector (e.g. `SELECT 'drop table' ...` is a read).
|
||||
* Handles single-quoted literals with '' escapes, double-quoted
|
||||
* identifiers, and dollar-quoted blocks ($$...$$ and $tag$...$tag$).
|
||||
*
|
||||
* @param {string} input
|
||||
* @returns {string}
|
||||
*/
|
||||
function stripSqlLiterals(input) {
|
||||
return String(input || '')
|
||||
.replace(/'(?:[^']|'')*'/g, "''")
|
||||
.replace(/"(?:[^"\\]|\\.)*"/g, '""')
|
||||
.replace(/(\$[A-Za-z_][A-Za-z0-9_]*\$|\$\$)[\s\S]*?\1/g, '$$$$');
|
||||
}
|
||||
|
||||
const SUDO_VALUE_FLAGS = new Set([
|
||||
'-u',
|
||||
'--user',
|
||||
'-g',
|
||||
'--group',
|
||||
'-U',
|
||||
'--other-user',
|
||||
'-p',
|
||||
'--prompt',
|
||||
'-C',
|
||||
'--close-from',
|
||||
'-D',
|
||||
'--chdir',
|
||||
'-h',
|
||||
'--host',
|
||||
'-r',
|
||||
'--role',
|
||||
'-t',
|
||||
'--type',
|
||||
'-T',
|
||||
'--command-timeout',
|
||||
]);
|
||||
|
||||
/**
|
||||
* Advance past `sudo`/`doas`/`env` wrappers including their flags and
|
||||
* `VAR=value` assignments, so `sudo -u postgres psql ...` and
|
||||
* `env PGUSER=postgres psql ...` still resolve to the real command.
|
||||
*
|
||||
* @param {string[]} tokens dequoted tokens for one segment
|
||||
* @returns {number} index of the real command token
|
||||
*/
|
||||
function unwrapLeadWrappers(tokens) {
|
||||
let index = 0;
|
||||
for (let guard = 0; guard < 4; guard += 1) {
|
||||
if (index >= tokens.length) return index;
|
||||
const base = commandBasename(tokens[index]);
|
||||
if (base === 'sudo' || base === 'doas') {
|
||||
index += 1;
|
||||
while (index < tokens.length) {
|
||||
const flag = tokens[index];
|
||||
if (flag === '--') {
|
||||
index += 1;
|
||||
break;
|
||||
}
|
||||
if (flag === '-' || !flag.startsWith('-')) break;
|
||||
if (SUDO_VALUE_FLAGS.has(flag)) {
|
||||
index += 2;
|
||||
continue;
|
||||
}
|
||||
if (/^--[^=]+=.*$/.test(flag)) {
|
||||
index += 1;
|
||||
continue;
|
||||
}
|
||||
index += 1;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (base === 'env') {
|
||||
index += 1;
|
||||
while (index < tokens.length) {
|
||||
const arg = tokens[index];
|
||||
if (arg === '--' || arg === '-' || arg === '-i' || arg === '--ignore-environment') {
|
||||
index += 1;
|
||||
continue;
|
||||
}
|
||||
if (arg === '-u' || arg === '--unset') {
|
||||
index += 2;
|
||||
continue;
|
||||
}
|
||||
if (arg === '-C' || arg === '--chdir') {
|
||||
index += 2;
|
||||
continue;
|
||||
}
|
||||
if (/^--unset=.*$/.test(arg) || /^--chdir=.*$/.test(arg) || /^--argv0=.*$/.test(arg)) {
|
||||
index += 1;
|
||||
continue;
|
||||
}
|
||||
if (arg.startsWith('-') && !/^[A-Za-z_][A-Za-z0-9_]*=/.test(arg)) {
|
||||
index += 1;
|
||||
continue;
|
||||
}
|
||||
if (/^[A-Za-z_][A-Za-z0-9_]*=/.test(arg)) {
|
||||
index += 1;
|
||||
continue;
|
||||
}
|
||||
break;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
break;
|
||||
}
|
||||
return index;
|
||||
}
|
||||
|
||||
/**
|
||||
* Detect destructive SQL passed as (possibly quoted) arguments to a known
|
||||
* SQL client. Operates on dequoted tokens from `quoteAwareSegments`, so
|
||||
* `psql -c "drop table users"` joins back to matchable text.
|
||||
*
|
||||
* @param {string[]} tokens dequoted tokens for one segment
|
||||
* @returns {boolean}
|
||||
*/
|
||||
function isDestructiveSqlClient(tokens) {
|
||||
if (!tokens || tokens.length === 0) return false;
|
||||
const start = unwrapLeadWrappers(tokens);
|
||||
if (start >= tokens.length) return false;
|
||||
if (!SQL_CLIENT_COMMANDS.has(commandBasename(tokens[start]))) return false;
|
||||
return DESTRUCTIVE_SQL_DD.test(stripSqlLiterals(tokens.slice(start).join(' ')));
|
||||
}
|
||||
|
||||
/**
|
||||
* Quote-aware destructive check: catches quoted command words, newline
|
||||
* separators, quoted `find -exec`, and `sh -c`/`bash -c` wrappers that evade
|
||||
@@ -374,10 +523,12 @@ function isDestructiveQuoteAware(raw, depth = 0) {
|
||||
if (tokens.length === 0) continue;
|
||||
if (isDestructiveRm(tokens)) return true;
|
||||
if (isDestructiveGit(tokens)) return true;
|
||||
if (isDestructiveSqlClient(tokens)) return true;
|
||||
if (isDestructiveFindExec(tokens.join(' '))) return true;
|
||||
const base = commandBasename(tokens[0]);
|
||||
const wi = unwrapLeadWrappers(tokens);
|
||||
const base = wi < tokens.length ? commandBasename(tokens[wi]) : '';
|
||||
if (SHELL_WRAPPERS.has(base)) {
|
||||
const ci = tokens.indexOf('-c');
|
||||
const ci = tokens.indexOf('-c', wi);
|
||||
if (ci !== -1 && tokens[ci + 1] && isDestructiveQuoteAware(tokens[ci + 1], depth + 1)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -1507,6 +1507,48 @@ function runTests() {
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
if (
|
||||
test('denies quoted destructive SQL passed to SQL clients (issue #3024)', () => {
|
||||
expectDestructiveDeny('psql -c "drop table users"', 'psql quoted drop table');
|
||||
expectDestructiveDeny("psql -c 'truncate audit_log'", 'psql quoted truncate');
|
||||
expectDestructiveDeny('mysql -e "delete from sessions"', 'mysql quoted delete');
|
||||
expectDestructiveDeny('sqlite3 app.db "DROP TABLE users"', 'sqlite3 quoted drop');
|
||||
})
|
||||
)
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
if (
|
||||
test('denies quoted destructive SQL through sudo/env wrappers', () => {
|
||||
expectDestructiveDeny('sudo -u postgres psql -c "drop table users"', 'sudo -u psql');
|
||||
expectDestructiveDeny('env PGUSER=postgres psql -c "drop table users"', 'env psql');
|
||||
expectDestructiveDeny('env PGPASSWORD=value psql -c "drop table users"', 'env PGPASSWORD psql');
|
||||
expectDestructiveDeny('env -C /tmp psql -c "drop table users"', 'env -C psql');
|
||||
expectDestructiveDeny('env --chdir /tmp psql -c "drop table users"', 'env --chdir psql');
|
||||
})
|
||||
)
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
if (
|
||||
test('denies destructive SQL through wrapper sh -c chains', () => {
|
||||
expectDestructiveDeny('sudo sh -c \'psql -c "drop table users"\'', 'sudo sh -c psql');
|
||||
expectDestructiveDeny('env sh -c \'psql -c "drop table users"\'', 'env sh -c psql');
|
||||
})
|
||||
)
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
if (
|
||||
test('allows SQL string literals and non-SQL clients mentioning SQL', () => {
|
||||
expectAllow('psql -c "SELECT \'drop table\' FROM audit_log"', 'SQL string literal');
|
||||
expectAllow('psql -c "SELECT $tag$drop table users$tag$ FROM t"', 'tagged dollar-quote literal');
|
||||
expectAllow('echo "drop table users"', 'echo SQL mention');
|
||||
})
|
||||
)
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
if (
|
||||
test('allows destructive SQL prose inside a quoted heredoc', () => {
|
||||
expectAllow(
|
||||
|
||||
Reference in New Issue
Block a user