Skip to content

fix: Eliminate command injection vulnerabilities in shell commands #488

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
txmyer-dev wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Eliminate command injection vulnerabilities in shell commands #488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
63 changes: 41 additions & 22 deletions Packs/pai-hook-system/src/hooks/UpdateTabTitle.hook.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
* - Includes 5-second stdin timeout
*/

import { execSync } from 'child_process';
import { execFileSync } from 'child_process';
import { readFileSync } from 'fs';
import { inference } from '../skills/CORE/Tools/Inference';
import { isValidTabSummary, getTabFallback } from './lib/response-format';
Expand Down Expand Up @@ -227,38 +227,50 @@ function setTabTitle(title: string, state: TabState = 'normal'): void {
// Add "…" suffix for active states
const titleWithSuffix = state !== 'normal' ? `${title}…` : title;
const truncated = titleWithSuffix.length > 50 ? titleWithSuffix.slice(0, 47) + '…' : titleWithSuffix;
const escaped = truncated.replace(/'/g, "'\\''");

// Check if we're in Kitty (TERM=xterm-kitty or KITTY_LISTEN_ON set)
const isKitty = process.env.TERM === 'xterm-kitty' || process.env.KITTY_LISTEN_ON;

if (isKitty) {
// Use Kitty remote control - works even without TTY
execSync(`kitty @ set-tab-title "${escaped}"`, { stdio: 'ignore', timeout: 2000 });
// SECURITY: Use execFileSync with argument array to prevent command injection
execFileSync('kitty', ['@', 'set-tab-title', truncated], { stdio: 'ignore', timeout: 2000 });

// Set color based on state
if (state === 'inference') {
// Purple for inference/AI thinking - active tab stays dark blue, inactive shows purple
execSync(
`kitten @ set-tab-color --self active_bg=${ACTIVE_TAB_BG} active_fg=${ACTIVE_TEXT} inactive_bg=${TAB_INFERENCE_BG} inactive_fg=${INACTIVE_TEXT}`,
{ stdio: 'ignore', timeout: 2000 }
);
execFileSync('kitten', [
'@', 'set-tab-color', '--self',
`active_bg=${ACTIVE_TAB_BG}`,
`active_fg=${ACTIVE_TEXT}`,
`inactive_bg=${TAB_INFERENCE_BG}`,
`inactive_fg=${INACTIVE_TEXT}`
], { stdio: 'ignore', timeout: 2000 });
console.error('[UpdateTabTitle] Set inference color (purple on inactive only)');
} else if (state === 'working') {
// Orange for actively working - active tab stays dark blue, inactive shows orange
execSync(
`kitten @ set-tab-color --self active_bg=${ACTIVE_TAB_BG} active_fg=${ACTIVE_TEXT} inactive_bg=${TAB_WORKING_BG} inactive_fg=${INACTIVE_TEXT}`,
{ stdio: 'ignore', timeout: 2000 }
);
execFileSync('kitten', [
'@', 'set-tab-color', '--self',
`active_bg=${ACTIVE_TAB_BG}`,
`active_fg=${ACTIVE_TEXT}`,
`inactive_bg=${TAB_WORKING_BG}`,
`inactive_fg=${INACTIVE_TEXT}`
], { stdio: 'ignore', timeout: 2000 });
console.error('[UpdateTabTitle] Set working color (orange on inactive only)');
}

console.error('[UpdateTabTitle] Set via Kitty remote control');
} else {
// Fallback to escape codes for other terminals
execSync(`printf '\\033]0;${escaped}\\007' >&2`, { stdio: ['pipe', 'pipe', 'inherit'] });
execSync(`printf '\\033]2;${escaped}\\007' >&2`, { stdio: ['pipe', 'pipe', 'inherit'] });
execSync(`printf '\\033]30;${escaped}\\007' >&2`, { stdio: ['pipe', 'pipe', 'inherit'] });
// SECURITY: Write escape sequences directly to stderr instead of using shell
const escapeSequences = [
`\x1b]0;${truncated}\x07`, // Set window title
`\x1b]2;${truncated}\x07`, // Set window title (xterm)
`\x1b]30;${truncated}\x07` // Set tab title (Konsole)
];
for (const seq of escapeSequences) {
process.stderr.write(seq);
}
}
} catch (err) {
console.error(`[UpdateTabTitle] Failed to set title: ${err}`);
Expand All @@ -267,18 +279,25 @@ function setTabTitle(title: string, state: TabState = 'normal'): void {

/**
* Send voice notification
* SECURITY: Uses fetch() instead of shell-interpolated curl to prevent command injection
*/
function announceVoice(summary: string): void {
async function announceVoice(summary: string): Promise<void> {
try {
// Summary already starts with gerund - use directly, capitalize first letter
const message = summary.charAt(0).toUpperCase() + summary.slice(1);
const escaped = message.replace(/"/g, '\\"');
execSync(
`curl -s -X POST http://localhost:8888/notify -H "Content-Type: application/json" -d '{"message": "${escaped}"}' > /dev/null 2>&1 &`,
{ stdio: 'ignore', timeout: 2000 }
);

// Use fetch with AbortController for timeout - no shell interpolation needed
const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), 2000);

await fetch('http://localhost:8888/notify', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ message }),
signal: controller.signal
}).finally(() => clearTimeout(timeoutId));
} catch {
// Voice server might not be running
// Voice server might not be running - silent failure is expected
}
}

Expand Down Expand Up @@ -324,7 +343,7 @@ async function main() {

// Voice announcement - validated to prevent garbage
if (isValidTabSummary(summary)) {
announceVoice(summary);
await announceVoice(summary);
console.error(`[UpdateTabTitle] Voice: "${summary}"`);
} else {
console.error(`[UpdateTabTitle] Skipped voice - invalid summary: "${summary}"`);
Expand Down
6 changes: 3 additions & 3 deletions Packs/pai-recon-skill/src/tools/BountyPrograms.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
* bun BountyPrograms.ts check example.com
*/

import { $ } from "bun";
import { mkdir } from "fs/promises";

interface BountyProgram {
name: string;
Expand Down Expand Up @@ -59,9 +59,9 @@ interface ChaosProgram {
}

async function ensureCacheDir(): Promise<void> {
const dir = Bun.file(CACHE_DIR);
try {
await $`mkdir -p ${CACHE_DIR}`.quiet();
// SECURITY: Use fs.mkdir instead of shell interpolation
await mkdir(CACHE_DIR, { recursive: true });
} catch {
// Directory might already exist
}
Expand Down
25 changes: 22 additions & 3 deletions Packs/pai-recon-skill/src/tools/WhoisParser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@
* const ipInfo = await whoisIP("1.2.3.4");
*/

import { $ } from "bun";
import { execFile } from "child_process";
import { promisify } from "util";

const execFileAsync = promisify(execFile);

export interface DomainWhoisInfo {
domain: string;
Expand Down Expand Up @@ -54,13 +57,29 @@ export interface IPWhoisInfo {
raw: string;
}

/**
* Validate WHOIS query input to prevent injection
*/
function validateWhoisQuery(query: string): void {
// Only allow valid domain/IP characters
const validPattern = /^[a-zA-Z0-9][a-zA-Z0-9.-]*[a-zA-Z0-9]$|^[a-zA-Z0-9]$/;
if (!validPattern.test(query)) {
throw new Error(`Invalid WHOIS query: contains disallowed characters`);
}
if (query.length > 253) {
throw new Error(`Invalid WHOIS query: too long`);
}
}

/**
* Execute WHOIS query
* SECURITY: Uses execFile with argument array to prevent command injection
*/
async function executeWhois(query: string): Promise<string> {
try {
const result = await $`whois ${query}`.text();
return result;
validateWhoisQuery(query);
const { stdout } = await execFileAsync('whois', [query], { timeout: 30000 });
return stdout;
} catch (error) {
throw new Error(
`WHOIS query failed: ${error instanceof Error ? error.message : "Unknown error"}`
Expand Down