[Mirror PR #3281] sec: Prevent Arbitrary Command Injection in Terminal Execution Helpers (#3202)

packages/data/src/hooks/_exec.test.ts

@@ -27,7 +27,7 @@ describe('execFileAsync', () => {
             stdout: 'hello stdout',
             stderr: 'hello stderr',
         });
-        expect(spy).toHaveBeenCalledWith('test-bin', ['arg1'], { cwd: '/tmp' }, expect.any(Function));
+        expect(spy).toHaveBeenCalledWith('test-bin', ['arg1'], { cwd: '/tmp', shell: false }, expect.any(Function));
     });
 
     it('should reject with error when execution fails', async () => {
@@ -42,4 +42,13 @@ describe('execFileAsync', () => {
 
         await expect(execFileAsync('test-bin', [])).rejects.toThrow('Spawn failed');
     });
+
+    it('should reject when file path contains null bytes', async () => {
+        await expect(execFileAsync('test-bin\0malicious', [])).rejects.toThrow('execFileAsync: file path contains null bytes');
+    });
+
+    it('should reject when an argument contains null bytes', async () => {
+        await expect(execFileAsync('test-bin', ['safe', 'malicious\0arg'])).rejects.toThrow('execFileAsync: argument contains null bytes');
+    });
 });
+

packages/data/src/hooks/_exec.ts

@@ -1,8 +1,21 @@
 import { execFile } from 'node:child_process';
 
 export const execFileAsync = (file: string, args: string[], opts?: any): Promise<{ stdout: string; stderr: string }> => {
+    if (!file || typeof file !== 'string') {
+        return Promise.reject(new TypeError('execFileAsync: file path must be a non-empty string'));
+    }
+    if (file.includes('\0')) {
+        return Promise.reject(new Error('execFileAsync: file path contains null bytes'));
+    }
+    if (Array.isArray(args)) {
+        for (const arg of args) {
+            if (typeof arg === 'string' && arg.includes('\0')) {
+                return Promise.reject(new Error('execFileAsync: argument contains null bytes'));
+            }
+        }
+    }
     return new Promise((resolve, reject) => {
-        execFile(file, args, opts, (err, stdout, stderr) => {
+        execFile(file, args, { shell: false, ...opts }, (err, stdout, stderr) => {
             if (err) reject(err);
             else resolve({ stdout: String(stdout), stderr: String(stderr) });
         });

packages/jsx/src/hooks/useSubprocess.test.ts

@@ -1,108 +1,116 @@
-import { EventEmitter } from 'node:events';
-import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
-import { useSubprocess } from './useSubprocess.js';
-import { setCurrentApp } from '../runtime.js';
-import { spawn } from 'node:child_process';
-
-vi.mock('node:child_process', () => ({
-    spawn: vi.fn(),
-}));
-
-const mockSpawn = vi.mocked(spawn);
-
-describe('useSubprocess', () => {
-    beforeEach(() => {
-        mockSpawn.mockReset();
-    });
-
-    afterEach(() => {
-        setCurrentApp(null);
-    });
-
-    it('spawns a subprocess with inherited stdio and returns the exit code', async () => {
-        const proc = new EventEmitter();
-
-        mockSpawn.mockReturnValue(proc as any);
-
-        const subprocess = useSubprocess();
-        const promise = subprocess.run(['git', 'status']);
-
-        proc.emit('close', 7);
-
-        const code = await promise;
-
-        expect(mockSpawn).toHaveBeenCalledWith('git', ['status'], {
-            stdio: 'inherit',
-        });
-        expect(code).toBe(7);
-    });
-
-    it('exits raw mode before spawning and restores the TUI after exit', async () => {
-        const app = {
-            terminal: {
-                exitRawMode: vi.fn(),
-                enterRawMode: vi.fn(),
-            },
-            screen: {
-                invalidate: vi.fn(),
-            },
-            requestRender: vi.fn(),
-        } as any;
-
-        setCurrentApp(app);
-
-        const proc = new EventEmitter();
-        mockSpawn.mockReturnValue(proc as any);
-
-        const subprocess = useSubprocess();
-        const promise = subprocess.run(['vim', 'file.txt']);
-
-        expect(app.terminal.exitRawMode).toHaveBeenCalledOnce();
-
-        proc.emit('close', 0);
-
-        const code = await promise;
-
-        expect(app.terminal.enterRawMode).toHaveBeenCalledOnce();
-        expect(app.screen.invalidate).toHaveBeenCalledOnce();
-        expect(app.requestRender).toHaveBeenCalledOnce();
-        expect(code).toBe(0);
-    });
-
-    it('restores raw mode and re-renders when the subprocess emits an error', async () => {
-        const app = {
-            terminal: {
-                exitRawMode: vi.fn(),
-                enterRawMode: vi.fn(),
-            },
-            screen: {
-                invalidate: vi.fn(),
-            },
-            requestRender: vi.fn(),
-        } as any;
-
-        setCurrentApp(app);
-
-        const proc = new EventEmitter();
-        mockSpawn.mockReturnValue(proc as any);
-
-        const subprocess = useSubprocess();
-        const promise = subprocess.run(['bad-command']);
-
-        proc.emit('error', new Error('spawn failed'));
-
-        await expect(promise).rejects.toThrow('spawn failed');
-
-        expect(app.terminal.enterRawMode).toHaveBeenCalledOnce();
-        expect(app.screen.invalidate).toHaveBeenCalledOnce();
-        expect(app.requestRender).toHaveBeenCalledOnce();
-    });
-
-    it('throws when command is empty', async () => {
-        const subprocess = useSubprocess();
-
-        await expect(subprocess.run([])).rejects.toThrow(
-            'useSubprocess.run requires a command',
-        );
-    });
-});
+import { EventEmitter } from 'node:events';
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { useSubprocess } from './useSubprocess.js';
+import { setCurrentApp } from '../runtime.js';
+import { spawn } from 'node:child_process';
+
+vi.mock('node:child_process', () => ({
+    spawn: vi.fn(),
+}));
+
+const mockSpawn = vi.mocked(spawn);
+
+describe('useSubprocess', () => {
+    beforeEach(() => {
+        mockSpawn.mockReset();
+    });
+
+    afterEach(() => {
+        setCurrentApp(null);
+    });
+
+    it('spawns a subprocess with inherited stdio and returns the exit code', async () => {
+        const proc = new EventEmitter();
+
+        mockSpawn.mockReturnValue(proc as any);
+
+        const subprocess = useSubprocess();
+        const promise = subprocess.run(['git', 'status']);
+
+        proc.emit('close', 7);
+
+        const code = await promise;
+
+        expect(mockSpawn).toHaveBeenCalledWith('git', ['status'], {
+            stdio: 'inherit',
+        });
+        expect(code).toBe(7);
+    });
+
+    it('exits raw mode before spawning and restores the TUI after exit', async () => {
+        const app = {
+            terminal: {
+                exitRawMode: vi.fn(),
+                enterRawMode: vi.fn(),
+            },
+            screen: {
+                invalidate: vi.fn(),
+            },
+            requestRender: vi.fn(),
+        } as any;
+
+        setCurrentApp(app);
+
+        const proc = new EventEmitter();
+        mockSpawn.mockReturnValue(proc as any);
+
+        const subprocess = useSubprocess();
+        const promise = subprocess.run(['vim', 'file.txt']);
+
+        expect(app.terminal.exitRawMode).toHaveBeenCalledOnce();
+
+        proc.emit('close', 0);
+
+        const code = await promise;
+
+        expect(app.terminal.enterRawMode).toHaveBeenCalledOnce();
+        expect(app.screen.invalidate).toHaveBeenCalledOnce();
+        expect(app.requestRender).toHaveBeenCalledOnce();
+        expect(code).toBe(0);
+    });
+
+    it('restores raw mode and re-renders when the subprocess emits an error', async () => {
+        const app = {
+            terminal: {
+                exitRawMode: vi.fn(),
+                enterRawMode: vi.fn(),
+            },
+            screen: {
+                invalidate: vi.fn(),
+            },
+            requestRender: vi.fn(),
+        } as any;
+
+        setCurrentApp(app);
+
+        const proc = new EventEmitter();
+        mockSpawn.mockReturnValue(proc as any);
+
+        const subprocess = useSubprocess();
+        const promise = subprocess.run(['bad-command']);
+
+        proc.emit('error', new Error('spawn failed'));
+
+        await expect(promise).rejects.toThrow('spawn failed');
+
+        expect(app.terminal.enterRawMode).toHaveBeenCalledOnce();
+        expect(app.screen.invalidate).toHaveBeenCalledOnce();
+        expect(app.requestRender).toHaveBeenCalledOnce();
+    });
+
+    it('throws when command is empty', async () => {
+        const subprocess = useSubprocess();
+
+        await expect(subprocess.run([])).rejects.toThrow(
+            'useSubprocess.run requires a command',
+        );
+    });
+
+    it('throws when command contains null bytes', async () => {
+        const subprocess = useSubprocess();
+
+        await expect(subprocess.run(['ls', 'dir\0malicious'])).rejects.toThrow(
+            'useSubprocess: command contains null bytes',
+        );
+    });
+});

packages/jsx/src/hooks/useSubprocess.ts

@@ -21,9 +21,14 @@ function spawnProcess(cmd: string[]): Promise<number> {
 
 export function useSubprocess(): UseSubprocessResult {
     async function run(cmd: string[]): Promise<number> {
-        if (cmd.length === 0) {
+        if (!cmd || cmd.length === 0) {
             throw new Error('useSubprocess.run requires a command');
         }
+        for (const part of cmd) {
+            if (typeof part === 'string' && part.includes('\0')) {
+                throw new Error('useSubprocess: command contains null bytes');
+            }
+        }
 
         const app = getCurrentApp();