Skip to content

Commit 2bf5919

Browse files
committed
Use spawn instead of exec to limit shell injection risk
This doesn't strictly fix an immediate vulnerability or risk (most cases where something could be injected here would also allow mitigations to handle this) but it does make it more difficult, and it's good practice.
1 parent cd8236c commit 2bf5919

File tree

1 file changed

+24
-12
lines changed

1 file changed

+24
-12
lines changed

src/interceptors/terminal/fresh-terminal-interceptor.ts

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import * as fs from 'fs';
33
import * as os from 'os';
44
import * as path from 'path';
55
import * as util from 'util';
6-
import { spawn, exec, ChildProcess, SpawnOptions } from 'child_process';
6+
import { spawn, ChildProcess, SpawnOptions } from 'child_process';
77
import * as GSettings from 'node-gsettings-wrapper';
88
import * as ensureCommandExists from 'command-exists';
99

@@ -35,13 +35,25 @@ interface SpawnArgs {
3535
skipStartupScripts?: true;
3636
}
3737

38-
const execAsync = (command: string): Promise<{ stdout: string, stderr: string }> => {
38+
// Spawn a command, and resolve with all stdout & stderr output as strings when it terminates
39+
const spawnAndCollectOutput = (command: string, args: string[] = []): Promise<{ stdout: string, stderr: string }> => {
3940
return new Promise((resolve, reject) => {
40-
const childProc = exec(command, (error, stdout, stderr) => {
41-
if (error) reject(error);
42-
else resolve({ stdout, stderr });
43-
});
41+
const childProc = spawn(command, args, { stdio: 'pipe' });
42+
const { stdout, stderr } = childProc;
43+
44+
const stdoutData: Buffer[] = [];
45+
stdout.on('data', (d) => stdoutData.push(d));
46+
const stderrData: Buffer[] = [];
47+
stderr.on('data', (d) => stderrData.push(d));
48+
4449
childProc.once('error', reject);
50+
childProc.once('exit', () => {
51+
// Note that we do _not_ check the error code
52+
resolve({
53+
stdout: Buffer.concat(stdoutData).toString(),
54+
stderr: Buffer.concat(stderrData).toString()
55+
});
56+
});
4557
});
4658
};
4759

@@ -107,7 +119,7 @@ const getLinuxTerminalCommand = async (): Promise<SpawnArgs | null> => {
107119
if (await commandExists('xterm')) return { command: 'xterm' };
108120

109121
return null;
110-
}
122+
};
111123

112124
const getXTerminalCommand = async (command = 'x-terminal-emulator'): Promise<SpawnArgs> => {
113125
// x-terminal-emulator is a wrapper/symlink to the terminal of choice.
@@ -118,7 +130,7 @@ const getXTerminalCommand = async (command = 'x-terminal-emulator'): Promise<Spa
118130
try {
119131
// Run the command with -h to get some output we can use to infer the terminal itself.
120132
// --version would be nice, but the debian wrapper ignores it. --help isn't supported by xterm.
121-
const { stdout } = await execAsync(`${command} -h`);
133+
const { stdout } = await spawnAndCollectOutput(command, ['-h']);
122134
const helpOutput = stdout.toLowerCase().replace(/[^\w\d]+/g, ' ');
123135

124136
if (helpOutput.includes('gnome terminal') && await commandExists('gnome-terminal')) {
@@ -138,13 +150,13 @@ const getXTerminalCommand = async (command = 'x-terminal-emulator'): Promise<Spa
138150
}
139151

140152
// If there's an error, or we just don't recognize the console, give up & run it directly
141-
return { command: 'x-terminal-emulator' };
153+
return { command };
142154
};
143155

144156
const getKonsoleTerminalCommand = async (command = 'konsole'): Promise<SpawnArgs> => {
145157
let extraArgs: string[] = [];
146158

147-
const { stdout } = await execAsync(`${command} --help`);
159+
const { stdout } = await spawnAndCollectOutput(command, ['--help']);
148160

149161
// Forces Konsole to run in the foreground, with no separate process
150162
// Seems to be well supported for a long time, but check just in case
@@ -158,7 +170,7 @@ const getKonsoleTerminalCommand = async (command = 'konsole'): Promise<SpawnArgs
158170
const getGnomeTerminalCommand = async (command = 'gnome-terminal'): Promise<SpawnArgs> => {
159171
let extraArgs: string[] = [];
160172

161-
const { stdout } = await execAsync(`${command} --help-all`);
173+
const { stdout } = await spawnAndCollectOutput(command, ['--help-all']);
162174

163175
// Officially supported option, but only supported in v3.28+
164176
if (stdout.includes('--wait')) {
@@ -180,7 +192,7 @@ const getGnomeTerminalCommand = async (command = 'gnome-terminal'): Promise<Spaw
180192
const getXfceTerminalCommand = async (command = 'xfce4-terminal'): Promise<SpawnArgs> => {
181193
let extraArgs: string[] = [];
182194

183-
const { stdout } = await execAsync(`${command} --help`);
195+
const { stdout } = await spawnAndCollectOutput(command, ['--help']);
184196

185197
// Disables the XFCE terminal server for this terminal, so it runs in the foreground.
186198
// Seems to be well supported for a long time, but check just in case

0 commit comments

Comments
 (0)