Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 89 additions & 0 deletions apps/cli/src/scan/__tests__/code-rules.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -355,3 +355,92 @@ describe('escaper matching does not over-reach', () => {
);
});
});

describe('shell', () => {
it('flags network output piped into a shell', () => {
expect(ruleIds('i.sh', 'curl -fsSL https://example.invalid/i.sh | bash')).toContain(
'sh-remote-script-execution',
);
expect(ruleIds('i.sh', 'wget -qO- https://example.invalid/i.sh | su' + 'do sh')).toContain(
'sh-remote-script-execution',
);
});

it('does not flag a download piped to something other than a shell', () => {
expect(ruleIds('i.sh', 'curl -fsSL https://example.invalid/v.json | jq -r .version')).toEqual(
[],
);
expect(ruleIds('i.sh', 'curl -fsSL https://example.invalid/f.tgz | sha256sum -c -')).toEqual([]);
});

it('flags eval handed an expansion', () => {
expect(ruleIds('i.sh', 'eval "$cmd"')).toContain('sh-eval-expansion');
expect(ruleIds('i.sh', 'eval $USER_SUPPLIED')).toContain('sh-eval-expansion');
expect(ruleIds('i.sh', 'eval "$(build_command "$1")"')).toContain('sh-eval-expansion');
});

// Bash's ordinary way to build a numeric range. Every expansion sits inside
// `$((…))`, where a `;` is a syntax error rather than a second command — and
// a line-wide `\beval\b.*\$` reported this 355 times in one real script.
it('does not flag the dynamic brace-range idiom', () => {
expect(ruleIds('i.sh', 'for r in $(eval echo {$(($k + 1))..$(($k + $n - 1))}); do')).toEqual([]);
expect(ruleIds('i.sh', 'for q in $(eval echo {1..$(($k + $l - 2))}); do')).toEqual([]);
});

it('does not flag the documented shell-init idiom', () => {
expect(ruleIds('i.sh', 'eval "$(dircolors -b)"')).toEqual([]);
expect(ruleIds('i.sh', 'eval "$(pyenv init -)"')).toEqual([]);
});

it('flags an unquoted expansion in a recursive remove, and not a quoted one', () => {
expect(ruleIds('i.sh', 'rm -rf $BUILD_DIR/output')).toContain(
'sh-unquoted-expansion-destructive',
);
// The correct form. `[^"'\n]*?` cannot cross the quote, so the match never
// reaches the expansion.
expect(ruleIds('i.sh', 'rm -rf "$BUILD_DIR/output"')).toEqual([]);
expect(ruleIds('i.sh', 'rm -rf "${BUILD_DIR:?}/output"')).toEqual([]);
});

it('flags disabled certificate verification over TLS', () => {
expect(ruleIds('i.sh', 'curl -k -L https://example.invalid/a.tgz > a.tgz')).toContain(
'sh-insecure-transport-flag',
);
expect(ruleIds('i.sh', 'wget --no-check-certificate https://example.invalid/a.tgz')).toContain(
'sh-insecure-transport-flag',
);
});

// `-k` skips a check that plain HTTP never performs. Reporting both put two
// findings on one line, one recommending a fix that would change nothing.
it('reports a plain-HTTP download once, as plaintext rather than a skipped check', () => {
expect(ruleIds('i.sh', 'curl -k -f http://example.invalid/a.gz > a.gz')).toEqual([
'sh-plaintext-download',
]);
});

it('does not flag plain HTTP to loopback', () => {
expect(ruleIds('i.sh', 'curl -s http://127.0.0.1:8080/health')).toEqual([]);
expect(ruleIds('i.sh', 'curl -s http://localhost:3000/ready')).toEqual([]);
});

it('flags world-writable permissions', () => {
expect(ruleIds('i.sh', 'chmod 777 /var/cache/app')).toContain('sh-world-writable-permissions');
expect(ruleIds('i.sh', 'chmod -R a+rwx /srv/data')).toContain('sh-world-writable-permissions');
expect(ruleIds('i.sh', 'chmod 0755 /usr/local/bin/app')).toEqual([]);
});

it('flags a predictable temp path unless mktemp made it', () => {
expect(ruleIds('i.sh', 'echo "$payload" > /tmp/app-build.log')).toContain(
'sh-predictable-temp-path',
);
const guarded = ['tmp=$(mktemp -d)', 'echo "$payload" > /tmp/app-build.log'].join('\n');
expect(ruleIds('i.sh', guarded)).toEqual([]);
});

it('does not apply shell rules to a language that merely mentions the same words', () => {
expect(ruleIds('a.js', 'const cmd = "curl -k https://x.invalid | bash";')).not.toContain(
'sh-remote-script-execution',
);
});
});
34 changes: 33 additions & 1 deletion apps/cli/src/scan/__tests__/engine.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import { describe, expect, it } from 'vitest';
import { parseFailOn } from '../../commands/scan.js';
import { collectSuppressions, languageOf, meetsFailThreshold, scanText } from '../engine.js';
import {
collectSuppressions,
languageOf,
languageOfShebang,
meetsFailThreshold,
scanText,
} from '../engine.js';
import { detectTyposquat, editDistance, scanPackageJson, scanRequirementsTxt } from '../manifest-rules.js';
import { isKnownPlaceholder, redactSecret } from '../secret-rules.js';
import type { ScanFinding } from '../types.js';
Expand All @@ -14,6 +20,32 @@ describe('language detection', () => {
});
});

describe('shebang detection', () => {
// An executable is named for the command it provides, not the language it is
// written in. `debtap` is 3,500 lines of bash with no extension, and
// extension-only detection scanned zero of them while reporting success.
it('reads the interpreter from a shebang', () => {
expect(languageOfShebang('#!/usr/bin/bash')).toBe('shell');
expect(languageOfShebang('#!/bin/sh')).toBe('shell');
expect(languageOfShebang('#!/usr/bin/env bash')).toBe('shell');
expect(languageOfShebang('#!/usr/bin/env python3')).toBe('python');
expect(languageOfShebang('#!/usr/bin/env node')).toBe('javascript');
expect(languageOfShebang('#!/usr/bin/ruby')).toBe('ruby');
});

it('tolerates a version suffix and extra whitespace', () => {
expect(languageOfShebang('#! /bin/bash')).toBe('shell');
expect(languageOfShebang('#!/usr/bin/python3.11')).toBe('python');
});

it('returns null for anything that is not a recognised interpreter', () => {
expect(languageOfShebang('#!/usr/bin/env perl')).toBeNull();
expect(languageOfShebang('# a comment, not a shebang')).toBeNull();
expect(languageOfShebang('')).toBeNull();
expect(languageOfShebang('\x7fELF\x02\x01')).toBeNull();
});
});

describe('secret redaction', () => {
it('never emits the matched credential', () => {
const redacted = redactSecret('AWS_ACCESS_KEY_ID=AKIAIOSFODNN7EXAMPLE');
Expand Down
134 changes: 134 additions & 0 deletions apps/cli/src/scan/code-rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,21 @@
const UNTRUSTED_JAVA =
/\bgetParameter\s*\(|\bgetQueryString\s*\(|\bgetHeader\s*\(|\bgetInputStream\s*\(|\bgetCookies\s*\(|\b@RequestParam\b|\b@PathVariable\b/;

/**
* Untrusted input to a shell script: what the caller controls.
*
* Positional parameters and `read` are the whole surface. Environment
* variables are deliberately absent — a script's own configuration arrives
* that way, so treating `$PREFIX` as attacker-controlled would mark every
* line of every installer.
*/
const UNTRUSTED_SH =
/\$\{?[1-9]\d*\b|\$[@*]|\$\{@\}|\bread\s+(?:-\S+\s+)*[A-Za-z_]\w*|\$\{?REPLY\b|\$\{?QUERY_STRING\b/;

export function untrustedPatternFor(language: ScanLanguage): RegExp {
switch (language) {
case 'shell':
return UNTRUSTED_SH;
case 'python':
return UNTRUSTED_PY;
case 'ruby':
Expand Down Expand Up @@ -709,6 +722,127 @@
pattern:
/\[\s*['"]__proto__['"]\s*\]|\bObject\s*\.\s*assign\s*\(\s*[\w.$]*\.prototype\b|\.\s*__proto__\s*=/,
},

// ── Shell ────────────────────────────────────────────────────────────────
//
// `shell` was a language the type system knew about and no rule targeted, so
// a repository written entirely in bash got secret detection and nothing
// else. Installers, CI helpers and packaging scripts are where a great deal
// of privileged work actually happens, and they run as whoever invoked them.
{
id: 'sh-remote-script-execution',
title: 'network output piped into a shell',
consequence:
'Whatever that URL serves at the moment this runs is executed as the invoking user. There is no version, no signature, and no review — a compromise of the host, or anyone able to answer for it, is a compromise of every machine that runs the script.',
cwe: 'CWE-494',
severity: 'high',
languages: ['shell'],
// The pipe must be the *next* thing: `curl -o f url && sh f` is a different
// (and checkable) shape, and `curl url | jq` is not an execution at all.
pattern: /\b(?:curl|wget)\b[^|\n]*\|\s*(?:sudo\s+(?:-\S+\s+)*)?(?:\/bin\/|\/usr\/bin\/)?(?:ba|da|k|z|a)?sh\b/,
// Nothing on this line can exonerate it. Integrity checking happens in a
// separate step by construction, so a window guard would only mislead.
guard: false,
},
{
id: 'sh-eval-expansion',
title: 'eval on an expanded string',
consequence:
'The expansion is re-parsed as shell source, so a `;` or `$(…)` anywhere in the value runs as a command rather than arriving as data.',
cwe: 'CWE-78',
severity: 'high',
languages: ['shell'],
// Matched against eval's *argument*, not against the rest of the line.
//
// `\beval\b.*\$` looks equivalent and is not: bash's ordinary dynamic-range
// idiom, `eval echo {$((k + 1))..$((k + n))}`, contains `$k` inside the
// arithmetic, so a line-wide search finds an expansion in a construct that
// cannot carry a command — `$((…))` is parsed as an expression, where a `;`
// is a syntax error rather than a second command. That spelling reported
// 355 findings in one 3,500-line script, all of them the same safe loop.
//
// What is left is the form where eval is handed a value directly —
// `eval "$cmd"`, `eval $cmd`, `eval "$(…)"` — which is the shape that
// actually re-parses untrusted text as source. `eval echo $x` is not
// covered; catching it without also catching the range idiom needs to know
// which expansions are arithmetic, which is parsing, not matching.
pattern: /\beval\s+(?:-\S+\s+)*(?:"\s*)?\$(?:\{?[A-Za-z_]\w*|\((?!\())/,
// The shell-init idiom `eval "$(tool init -)"` is the documented interface
// of most version managers. It is still eval of program output, but the
// program is a fixed local binary, and flagging it reports every developer
// dotfile in existence.
lineGuard:
/\beval\s+"?\$\(\s*(?:ssh-agent|dircolors|direnv|rbenv|pyenv|nodenv|goenv|jenv|tfenv|opam|luarocks|conda|mamba|zoxide|starship|mise|asdf|fnm|nvm|brew|thefuck|register-python-argcomplete|_\w+_completion)\b/,
},
{
id: 'sh-unquoted-expansion-destructive',
title: 'unquoted expansion in a destructive command',
consequence:
'An unquoted expansion is word-split and glob-expanded before the command sees it. A value with a space removes two paths instead of one; an empty value removes the argument entirely, which is how `rm -rf $DIR/` becomes `rm -rf /`.',
cwe: 'CWE-78',
severity: 'high',
languages: ['shell'],
// `[^"'\n]*?` cannot cross a quote, so `rm -rf "$dir"` — the correct form —
// never reaches the `$` and never matches. Only a genuinely bare expansion
// does. Restricted to recursive/forced removal: a bare `$f` in `rm $f` is
// sloppy, but it is not the shape that erases a filesystem.
pattern: /\brm\s+(?:-[a-zA-Z-]*[rRf][a-zA-Z-]*\s+)+[^"'\n]*?\$\{?[A-Za-z_]/,
},
{
id: 'sh-insecure-transport-flag',
title: 'certificate verification disabled',
consequence:
'Anyone positioned between this host and the server can substitute the response. When the response is a package, a key or a script, that is remote code execution with the transport doing nothing to stop it.',
cwe: 'CWE-295',
severity: 'high',
languages: ['shell'],
pattern:
/\b(?:curl|wget)\b[^\n|]*(?:\s-k(?=\s|$)|\s--insecure\b|\s--no-check-certificate\b)/,
// Over plain HTTP there is no certificate to skip, so the flag is inert and
// this rule has nothing to say — `sh-plaintext-download` is the finding
// that fits. Reporting both put two entries on one line, one of which
// recommended a fix that would change nothing.
lineGuard: /^(?!.*https:\/\/).*\bhttp:\/\//,
},
{
id: 'sh-plaintext-download',
title: 'download over plain HTTP',
consequence:
'The response arrives unauthenticated over a channel any intermediary can rewrite. Where the payload is an archive, a package list or a key, substituting it is straightforward and leaves nothing for the script to notice.',
cwe: 'CWE-319',
severity: 'high',
languages: ['shell'],
pattern: /\b(?:curl|wget)\b[^\n|]*\bhttp:\/\//,
// Loopback and link-local are not carried over a network anyone can sit on.
lineGuard:
/http:\/\/(?:localhost|127\.0\.0\.1|0\.0\.0\.0|\[::1\]|169\.254\.|host\.docker\.internal)\b/,
},
{
id: 'sh-world-writable-permissions',
title: 'world-writable permissions',
consequence:
'Any local account can rewrite the file. If it is a script, a config or anything on a privileged path, the next process to read it runs someone else’s content.',
cwe: 'CWE-732',
severity: 'medium',
languages: ['shell'],
pattern: /\bchmod\s+(?:-[a-zA-Z-]+\s+)*(?:0?777|a\+rwx|ugo\+rwx|a=rwx)\b/,
},
{
id: 'sh-predictable-temp-path',
title: 'predictable temporary file',
consequence:
'The name is guessable, so a local attacker can create it first — as a symlink to somewhere that matters — and the script writes through it with its own privileges.',
cwe: 'CWE-377',
severity: 'medium',
languages: ['shell'],
// Redirection or an explicit write into a literal `/tmp` path. A `$$` or
// `$RANDOM` suffix is still predictable, so it is not treated as a fix;
// `mktemp` is, and it is the guard below.
pattern: /(?:>{1,2}\s*|\b(?:tee|touch|cp|mv|install)\s+(?:-\S+\s+)*)\/tmp\/[\w.$-]+/,
guard: /\bmktemp\b/,
guardBack: 6,
guardForward: 2,
},
];

/**
Expand Down
66 changes: 63 additions & 3 deletions apps/cli/src/scan/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
* without three implementations drifting apart.
*/

import { closeSync, fstatSync, openSync, readdirSync, readFileSync, statSync } from 'node:fs';
import {
closeSync, fstatSync, openSync, readdirSync, readFileSync, readSync, statSync,
} from 'node:fs';
import { basename, dirname, extname, join, relative, sep } from 'node:path';
import { CODE_RULES, evaluateRule, proseLines } from './code-rules.js';
import { scanPackageJson, scanRequirementsTxt } from './manifest-rules.js';
Expand Down Expand Up @@ -50,6 +52,43 @@ export function languageOf(filename: string): ScanLanguage {
return LANGUAGE_BY_EXTENSION[extname(filename).toLowerCase()] ?? 'other';
}

/** Interpreters worth recognising, by the language their scripts are written in. */
const LANGUAGE_BY_INTERPRETER: Record<string, ScanLanguage> = {
sh: 'shell', bash: 'shell', zsh: 'shell', dash: 'shell', ksh: 'shell', ash: 'shell',
python: 'python', python2: 'python', python3: 'python',
ruby: 'ruby',
node: 'javascript', nodejs: 'javascript', deno: 'javascript', bun: 'javascript',
php: 'php',
};

/**
* The language a `#!` line declares, or `null` if the line is not a shebang.
*
* An executable in a repository root is routinely named for the command it
* provides rather than the language it is written in — `debtap`, `configure`,
* `gradlew`. Extension-based detection skips every one of them, which is worst
* exactly where it matters: a project whose entire source is one extensionless
* script gets a clean scan because nothing was read.
*
* The shebang is the authoritative answer to a question the filename cannot
* answer, and the kernel already treats it that way.
*/
export function languageOfShebang(firstLine: string): ScanLanguage | null {
const match = /^#!\s*(\S+)(?:\s+(\S+))?/.exec(firstLine);
if (!match) return null;

// `#!/usr/bin/env bash` names the interpreter in the argument, not the path.
const command = basename(match[1]!);
const name = command === 'env' && match[2] ? basename(match[2]) : command;

const exact = LANGUAGE_BY_INTERPRETER[name];
if (exact) return exact;

// `python3.11` and `bash5` are the same interpreters with a version glued on.
const stripped = name.replace(/[\d.]+$/, '');
return (stripped ? LANGUAGE_BY_INTERPRETER[stripped] : undefined) ?? null;
}

export interface ScanOptions {
/** Skip files larger than this. Defaults to 1 MiB. */
maxFileBytes?: number;
Expand Down Expand Up @@ -272,7 +311,16 @@ export function scanPath(targetPath: string, options: ScanOptions = {}): ScanRep
const isManifest = filename === 'package.json' || filename === 'requirements.txt';
const scannable = SCAN_EXTENSIONS.has(extension) || filename.startsWith('.env');

if (!scannable && !isManifest) {
// A file with no extension gets one question asked of it before being
// dismissed: does it start with a shebang? Executables are habitually named
// for what they do rather than what they are written in, and skipping them
// silently is how a repository whose only source file is `debtap` scans
// clean. Files that carry an unrecognised extension are still skipped —
// `.png` is not a script, and sniffing every one of them would mean reading
// the whole tree.
const mayDeclareInterpreter = !scannable && !isManifest && extension === '';

if (!scannable && !isManifest && !mayDeclareInterpreter) {
recordSensitiveFile(filename, relativePath, findings, []);
return;
}
Expand All @@ -290,6 +338,7 @@ export function scanPath(targetPath: string, options: ScanOptions = {}): ScanRep
// right in its own walker.
let text: string;
let handle: number;
let declared: ScanLanguage | null = null;
try {
handle = openSync(fullPath, 'r');
} catch {
Expand All @@ -299,6 +348,17 @@ export function scanPath(targetPath: string, options: ScanOptions = {}): ScanRep

try {
if (fstatSync(handle).size > maxFileBytes) return;

// Sniff the shebang from a short prefix rather than the whole file, so an
// extensionless blob — a checked-in binary, a data file — costs one small
// read instead of a megabyte decoded as UTF-8 and thrown away.
if (mayDeclareInterpreter) {
const prefix = Buffer.alloc(128);
const read = readSync(handle, prefix, 0, prefix.length, 0);
declared = languageOfShebang(prefix.subarray(0, read).toString('utf-8').split('\n', 1)[0] ?? '');
if (!declared) return;
}

text = readFileSync(handle, 'utf-8');
} catch {
unreadable.push(relativePath);
Expand All @@ -316,7 +376,7 @@ export function scanPath(targetPath: string, options: ScanOptions = {}): ScanRep
suppressed += collectSuppressions(text.split('\n')).count;

const fileFindings = [
...scanText(relativePath, text, languageOf(filename)),
...scanText(relativePath, text, declared ?? languageOf(filename)),
...(isManifest ? scanManifest(relativePath, filename, text) : []),
];

Expand Down
Loading