fix: removed unsafe shell when executing process

Ref: PNX-3671 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>

Author: Akos Kitta

arduino-ide-extension/package.json

@@ -110,11 +110,9 @@
  "devDependencies": {
  "@octokit/rest": "^18.12.0",
  "@types/chai": "^4.2.7",
- "@types/chai-string": "^1.4.2",
  "@types/mocha": "^5.2.7",
  "@types/react-window": "^1.8.5",
  "chai": "^4.2.0",
- "chai-string": "^1.5.0",
  "decompress": "^4.2.0",
  "decompress-tarbz2": "^4.1.1",
  "decompress-targz": "^4.1.1",

arduino-ide-extension/src/common/protocol/executable-service.ts

@@ -5,6 +5,5 @@ export interface ExecutableService {
  clangdUri: string;
  cliUri: string;
  lsUri: string;
- fwuploaderUri: string;
  }>;
 }

arduino-ide-extension/src/node/arduino-daemon-impl.ts

@@ -44,7 +44,6 @@ export class ArduinoDaemonImpl
 
  private _running = false;
  private _port = new Deferred<string>();
- private _execPath: string | undefined;
 
  // Backend application lifecycle.
 
@@ -68,7 +67,7 @@ export class ArduinoDaemonImpl
  async start(): Promise<string> {
  try {
  this.toDispose.dispose(); // This will `kill` the previously started daemon process, if any.
- const cliPath = await this.getExecPath();
+ const cliPath = this.getExecPath();
  this.onData(`Starting daemon from ${cliPath}...`);
  const { daemon, port } = await this.spawnDaemonProcess();
  // Watchdog process for terminating the daemon process when the backend app terminates.
@@ -132,12 +131,8 @@ export class ArduinoDaemonImpl
  return this.onDaemonStoppedEmitter.event;
  }
 
- async getExecPath(): Promise<string> {
- if (this._execPath) {
- return this._execPath;
- }
- this._execPath = await getExecPath('arduino-cli', this.onError.bind(this));
- return this._execPath;
+ getExecPath(): string {
+ return getExecPath('arduino-cli');
  }
 
  protected async getSpawnArgs(): Promise<string[]> {
@@ -151,7 +146,7 @@ export class ArduinoDaemonImpl
  '--port',
  '0',
  '--config-file',
- `"${cliConfigPath}"`,
+ cliConfigPath,
  '-v',
  ];
  if (debug) {
@@ -173,10 +168,8 @@ export class ArduinoDaemonImpl
  daemon: ChildProcess;
  port: string;
  }> {
- const [cliPath, args] = await Promise.all([
- this.getExecPath(),
- this.getSpawnArgs(),
- ]);
+ const args = await this.getSpawnArgs();
+ const cliPath = this.getExecPath();
  const ready = new Deferred<{ daemon: ChildProcess; port: string }>();
  const options = { shell: true };
  const daemon = spawn(`"${cliPath}"`, args, options);

arduino-ide-extension/src/node/arduino-firmware-uploader-impl.ts

@@ -1,45 +1,22 @@
+import { ILogger } from '@theia/core/lib/common/logger';
+import { inject, injectable, named } from '@theia/core/shared/inversify';
+import type { Port } from '../common/protocol';
 import {
  ArduinoFirmwareUploader,
  FirmwareInfo,
 } from '../common/protocol/arduino-firmware-uploader';
-import { injectable, inject, named } from '@theia/core/shared/inversify';
-import { ExecutableService, Port } from '../common/protocol';
 import { getExecPath, spawnCommand } from './exec-util';
-import { ILogger } from '@theia/core/lib/common/logger';
 import { MonitorManager } from './monitor-manager';
 
 @injectable()
 export class ArduinoFirmwareUploaderImpl implements ArduinoFirmwareUploader {
- @inject(ExecutableService)
- protected executableService: ExecutableService;
-
- protected _execPath: string | undefined;
-
  @inject(ILogger)
  @named('fwuploader')
- protected readonly logger: ILogger;
-
+ private readonly logger: ILogger;
  @inject(MonitorManager)
- protected readonly monitorManager: MonitorManager;
-
- protected onError(error: any): void {
- this.logger.error(error);
- }
-
- async getExecPath(): Promise<string> {
- if (this._execPath) {
- return this._execPath;
- }
- this._execPath = await getExecPath('arduino-fwuploader');
- return this._execPath;
- }
-
- async runCommand(args: string[]): Promise<any> {
- const execPath = await this.getExecPath();
- return await spawnCommand(`"${execPath}"`, args, this.onError.bind(this));
- }
+ private readonly monitorManager: MonitorManager;
 
- async uploadCertificates(command: string): Promise<any> {
+ async uploadCertificates(command: string): Promise<string> {
  return await this.runCommand(['certificates', 'flash', command]);
  }
 
@@ -70,14 +47,13 @@ export class ArduinoFirmwareUploaderImpl implements ArduinoFirmwareUploader {
  }
 
  async flash(firmware: FirmwareInfo, port: Port): Promise<string> {
- let output;
  const board = {
  name: firmware.board_name,
  fqbn: firmware.board_fqbn,
  };
  try {
  await this.monitorManager.notifyUploadStarted(board.fqbn, port);
- output = await this.runCommand([
+ const output = await this.runCommand([
  'firmware',
  'flash',
  '--fqbn',
@@ -87,11 +63,18 @@ export class ArduinoFirmwareUploaderImpl implements ArduinoFirmwareUploader {
  '--module',
  `${firmware.module}@${firmware.firmware_version}`,
  ]);
- } catch (e) {
- throw e;
+ return output;
  } finally {
  await this.monitorManager.notifyUploadFinished(board.fqbn, port);
- return output;
  }
  }
+
+ private onError(error: Error): void {
+ this.logger.error(error);
+ }
+
+ private async runCommand(args: string[]): Promise<string> {
+ const execPath = getExecPath('arduino-fwuploader');
+ return await spawnCommand(execPath, args, this.onError.bind(this));
+ }
 }

arduino-ide-extension/src/node/clang-formatter.ts

@@ -1,4 +1,3 @@
-import * as os from 'node:os';
 import { EnvVariablesServer } from '@theia/core/lib/common/env-variables';
 import { MaybePromise } from '@theia/core/lib/common/types';
 import { FileUri } from '@theia/core/lib/node/file-uri';
@@ -15,7 +14,7 @@ export class ClangFormatter implements Formatter {
  private readonly configService: ConfigService;
 
  @inject(EnvVariablesServer)
- private readonly envVariableServer: EnvVariablesServer;
+ private readonly envVariablesServer: EnvVariablesServer;
 
  async format({
  content,
@@ -26,26 +25,19 @@ export class ClangFormatter implements Formatter {
  formatterConfigFolderUris: string[];
  options?: FormatterOptions;
  }): Promise<string> {
- const [execPath, style] = await Promise.all([
- this.execPath(),
- this.style(formatterConfigFolderUris, options),
- ]);
+ const execPath = this.execPath();
+ const args = await this.styleArgs(formatterConfigFolderUris, options);
  const formatted = await spawnCommand(
- `"${execPath}"`,
- [style],
+ execPath,
+ args,
  console.error,
  content
  );
  return formatted;
  }
 
- private _execPath: string | undefined;
- private async execPath(): Promise<string> {
- if (this._execPath) {
- return this._execPath;
- }
- this._execPath = await getExecPath('clang-format');
- return this._execPath;
+ private execPath(): string {
+ return getExecPath('clang-format');
  }
 
  /**
@@ -60,10 +52,10 @@ export class ClangFormatter implements Formatter {
  *
  * See: https://github.com/arduino/arduino-ide/issues/566
  */
- private async style(
+ private async styleArgs(
  formatterConfigFolderUris: string[],
  options?: FormatterOptions
- ): Promise<string> {
+ ): Promise<string[]> {
  const clangFormatPaths = await Promise.all([
  ...formatterConfigFolderUris.map((uri) => this.clangConfigPath(uri)),
  this.clangConfigPath(this.configDirPath()),
@@ -72,11 +64,11 @@ export class ClangFormatter implements Formatter {
  const first = clangFormatPaths.filter(Boolean).shift();
  if (first) {
  console.debug(
- `Using ${ClangFormatFile} style configuration from '${first}'.`
+ `Using ${clangFormatFilename} style configuration from '${first}'.`
  );
- return `-style=file:"${first}"`;
+ return ['-style', `file:${first}`];
  }
- return `-style="${style(toClangOptions(options))}"`;
+ return ['-style', style(toClangOptions(options))];
  }
 
  private async dataDirPath(): Promise<string | undefined> {
@@ -88,7 +80,7 @@ export class ClangFormatter implements Formatter {
  }
 
  private async configDirPath(): Promise<string> {
- const configDirUri = await this.envVariableServer.getConfigDirUri();
+ const configDirUri = await this.envVariablesServer.getConfigDirUri();
  return FileUri.fsPath(configDirUri);
  }
 
@@ -100,7 +92,7 @@ export class ClangFormatter implements Formatter {
  return undefined;
  }
  const folderPath = FileUri.fsPath(uri);
- const clangFormatPath = join(folderPath, ClangFormatFile);
+ const clangFormatPath = join(folderPath, clangFormatFilename);
  try {
  await fs.access(clangFormatPath, constants.R_OK);
  return clangFormatPath;
@@ -115,7 +107,7 @@ interface ClangFormatOptions {
  readonly TabWidth: number;
 }
 
-const ClangFormatFile = '.clang-format';
+export const clangFormatFilename = '.clang-format';
 
 function toClangOptions(
  options?: FormatterOptions | undefined
@@ -129,30 +121,14 @@ function toClangOptions(
  return { UseTab: 'Never', TabWidth: 2 };
 }
 
-export function style({ TabWidth, UseTab }: ClangFormatOptions): string {
- let styleArgument = JSON.stringify(styleJson({ TabWidth, UseTab })).replace(
- /[\\"]/g,
- '\\$&'
- );
- if (os.platform() === 'win32') {
- // Windows command interpreter does not use backslash escapes. This causes the argument to have alternate quoted and
- // unquoted sections.
- // Special characters in the unquoted sections must be caret escaped.
- const styleArgumentSplit = styleArgument.split('"');
- for (let i = 1; i < styleArgumentSplit.length; i += 2) {
- styleArgumentSplit[i] = styleArgumentSplit[i].replace(/[<>^|]/g, '^$&');
- }
-
- styleArgument = styleArgumentSplit.join('"');
- }
-
- return styleArgument;
+function style({ TabWidth, UseTab }: ClangFormatOptions): string {
+ return stringifyConfiguration(styleJson({ TabWidth, UseTab }));
 }
 
 function styleJson({
  TabWidth,
  UseTab,
-}: ClangFormatOptions): Record<string, unknown> {
+}: ClangFormatOptions): ClangConfiguration {
  // Source: https://github.com/arduino/tooling-project-assets/tree/main/other/clang-format-configuration
  const defaultConfig = require('../../src/node/default-formatter-config.json');
  return {
@@ -161,3 +137,26 @@ function styleJson({
  UseTab,
  };
 }
+
+export type ClangStyleValue =
+ | string
+ | number
+ | boolean
+ | ClangStyleValue[]
+ | { [key: string]: ClangStyleValue };
+export type ClangConfiguration = Record<string, ClangStyleValue>;
+
+export function stringifyConfiguration(
+ value: ClangStyleValue | ClangConfiguration
+): string {
+ if (Array.isArray(value)) {
+ return `[${value.map((item) => stringifyConfiguration(item)).join(', ')}]`;
+ } else if (typeof value === 'object') {
+ return `{${Object.entries(value)
+ .map(([key, v]) => `${key}: ${stringifyConfiguration(v)}`)
+ .join(', ')}}`;
+ } else if (typeof value === 'string') {
+ return `'${value}'`;
+ }
+ return String(value);
+}

arduino-ide-extension/src/node/config-service-impl.ts

@@ -222,8 +222,8 @@ export class ConfigServiceImpl
  }
 
  private async getFallbackCliConfig(): Promise<DefaultCliConfig> {
- const cliPath = await this.daemon.getExecPath();
- const rawJson = await spawnCommand(`"${cliPath}"`, [
+ const cliPath = this.daemon.getExecPath();
+ const rawJson = await spawnCommand(cliPath, [
  'config',
  'dump',
  'format',
@@ -233,13 +233,8 @@ export class ConfigServiceImpl
  }
 
  private async initCliConfigTo(fsPathToDir: string): Promise<void> {
- const cliPath = await this.daemon.getExecPath();
- await spawnCommand(`"${cliPath}"`, [
- 'config',
- 'init',
- '--dest-dir',
- `"${fsPathToDir}"`,
- ]);
+ const cliPath = this.daemon.getExecPath();
+ await spawnCommand(cliPath, ['config', 'init', '--dest-dir', fsPathToDir]);
  }
 
  private async mapCliConfigToAppConfig(