Fix arbitrary code execution vulnerability

src/get-default-printer/get-default-printer.spec.ts

@@ -1,5 +1,5 @@
 import getDefaultPrinter from "./get-default-printer";
-import execAsync from "../utils/exec-async";
+import { execFileAsync } from "../utils/exec-async";
 import { Printer } from "../types";
 
 jest.mock("../utils/exec-async");
@@ -20,11 +20,11 @@ Interface: /etc/cups/ppd/Virtual_PDF_Printer.ppd
 
 afterEach(() => {
   // restore the original implementation.
-  execAsync.mockRestore();
+  execFileAsync.mockRestore();
 });
 
 it("returns the system default printer", async () => {
-  execAsync
+  execFileAsync
     .mockImplementationOnce(() =>
       Promise.resolve({ stdout: defaultPrinterStdout })
     )
@@ -41,18 +41,18 @@ it("returns the system default printer", async () => {
   };
 
   await expect(getDefaultPrinter()).resolves.toEqual(expected);
-  await expect(execAsync).toBeCalledWith("lpstat -d");
+  await expect(execFileAsync).toBeCalledWith("lpstat", ["-d"]);
 });
 
 it("returns null when the default printer is not defined", async () => {
-  execAsync.mockImplementation(() =>
+  execFileAsync.mockImplementation(() =>
     Promise.resolve({ stdout: "no system default destination" })
   );
 
   await expect(getDefaultPrinter()).resolves.toBeNull();
 });
 
 it("fails with an error", async () => {
-  execAsync.mockImplementation(() => Promise.reject("error"));
+  execFileAsync.mockImplementation(() => Promise.reject("error"));
   await expect(getDefaultPrinter()).rejects.toMatch("error");
 });

src/get-default-printer/get-default-printer.ts

@@ -1,10 +1,10 @@
 import { Printer } from "../types";
-import execAsync from "../utils/exec-async";
+import { execFileAsync } from "../utils/exec-async";
 import parsePrinterAttribute from "../utils/parse-printer-attribute";
 
 export default async function getDefaultPrinter(): Promise<Printer | null> {
   try {
-    const { stdout } = await execAsync("lpstat -d");
+    const { stdout } = await execFileAsync("lpstat", ["-d"]);
     const printer = getPrinterName(stdout);
     if (!printer) return null;
     return await getPrinterData(printer);
@@ -19,7 +19,7 @@ function getPrinterName(output: string): string {
 }
 
 async function getPrinterData(printer: string): Promise<Printer> {
-  const { stdout } = await execAsync(`lpstat -lp ${printer}`);
+  const { stdout } = await execFileAsync("lpstat", ["-lp", printer]);
   return {
     printer,
     status: stdout.split(/.*is\s(\w+)\..*/gm)[1],

src/get-printers/get-printers.spec.ts

@@ -1,5 +1,5 @@
 import getPrinters from "./get-printers";
-import execAsync from "../utils/exec-async";
+import { execFileAsync } from "../utils/exec-async";
 import { Printer } from "../types";
 
 jest.mock("../utils/exec-async");
@@ -31,11 +31,11 @@ After fault: continue
 
 afterEach(() => {
   // restore the original implementation.
-  execAsync.mockRestore();
+  execFileAsync.mockRestore();
 });
 
 it("return a list of available printers", async () => {
-  execAsync.mockImplementation(() => Promise.resolve({ stdout }));
+  execFileAsync.mockImplementation(() => Promise.resolve({ stdout }));
 
   const expected: Printer[] = [
     {
@@ -58,14 +58,14 @@ it("return a list of available printers", async () => {
 });
 
 it("return an empty list when there are no printers installed.", async () => {
-  execAsync.mockImplementation(() =>
+  execFileAsync.mockImplementation(() =>
     Promise.resolve({ stdout: "lpstat: No destinations added." })
   );
 
   await expect(getPrinters()).resolves.toEqual([]);
 });
 
 it("fails with an error", async () => {
-  execAsync.mockImplementation(() => Promise.reject("error"));
+  execFileAsync.mockImplementation(() => Promise.reject("error"));
   await expect(getPrinters()).rejects.toMatch("error");
 });

src/get-printers/get-printers.ts

@@ -1,10 +1,10 @@
 import { Printer } from "../types";
-import execAsync from "../utils/exec-async";
+import { execFileAsync } from "../utils/exec-async";
 import parsePrinterAttribute from "../utils/parse-printer-attribute";
 
 export default async function getPrinters(): Promise<Printer[]> {
   try {
-    const { stdout } = await execAsync("lpstat -lp");
+    const { stdout } = await execFileAsync("lpstat", ["-lp"]);
 
     const isThereAnyPrinter = stdout.match("printer");
     if (!isThereAnyPrinter) return [];

src/print/print.spec.ts

@@ -1,5 +1,5 @@
 import { existsSync } from "fs";
-import execAsync from "../utils/exec-async";
+import { execFileAsync } from "../utils/exec-async";
 import print from "./print";
 
 jest.mock("fs");
@@ -9,15 +9,15 @@ jest.mock("../utils/exec-async");
 beforeEach(() => {
   // override the implementations
   existsSync.mockImplementation(() => true);
-  execAsync.mockImplementation(() =>
+  execFileAsync.mockImplementation(() =>
     Promise.resolve({ stdout: "request id is myDummyPrinter-15 (1 file(s))" })
   );
 });
 
 afterEach(() => {
   // restore the original implementations
   existsSync.mockRestore();
-  execAsync.mockRestore();
+  execFileAsync.mockRestore();
 });
 
 it("throws when no file is specified.", async () => {
@@ -35,7 +35,7 @@ it("sends the PDF file to the default printer", async () => {
 
   await print(filename);
 
-  expect(execAsync).toHaveBeenCalledWith(`lp '${filename}'`);
+  expect(execFileAsync).toHaveBeenCalledWith("lp", [filename]);
 });
 
 it("sends PDF file to the specific printer", async () => {
@@ -44,7 +44,7 @@ it("sends PDF file to the specific printer", async () => {
 
   await print(filename, printer);
 
-  expect(execAsync).toHaveBeenCalledWith(`lp '${filename}' -d ${printer}`);
+  expect(execFileAsync).toHaveBeenCalledWith("lp", [filename, "-d", printer]);
 });
 
 it("allows to pass other print options", async () => {
@@ -54,9 +54,14 @@ it("allows to pass other print options", async () => {
 
   await print(filename, printer, options);
 
-  expect(execAsync).toHaveBeenCalledWith(
-    `lp '${filename}' -d ${printer} -o landscape -o fit-to-page -o media=A4`
-  );
+  expect(execFileAsync).toHaveBeenCalledWith("lp", [
+    filename,
+    "-d",
+    printer,
+    "-o landscape",
+    "-o fit-to-page",
+    "-o media=A4",
+  ]);
 });
 
 it("allows to pass options but omit the printer name", async () => {
@@ -65,9 +70,12 @@ it("allows to pass options but omit the printer name", async () => {
 
   await print(filename, undefined, options);
 
-  expect(execAsync).toHaveBeenCalledWith(
-    `lp '${filename}' -o landscape -o fit-to-page -o media=A4`
-  );
+  expect(execFileAsync).toHaveBeenCalledWith("lp", [
+    filename,
+    "-o landscape",
+    "-o fit-to-page",
+    "-o media=A4",
+  ]);
 });
 
 it("throws if options passed not as an array", async () => {

src/print/print.ts

@@ -1,6 +1,6 @@
 import fs from "fs";
 import { ExecResponse } from "../types";
-import execAsync from "../utils/exec-async";
+import { execFileAsync } from "../utils/exec-async";
 
 export default async function print(
   file: string,
@@ -10,7 +10,7 @@ export default async function print(
   if (!file) throw "No file specified";
   if (!fs.existsSync(file)) throw "No such file";
 
-  const args = [`'${file}'`];
+  const args = [file];
 
   if (printer) {
     args.push("-d", printer);
@@ -19,8 +19,8 @@ export default async function print(
   if (options) {
     if (!Array.isArray(options)) throw "options should be an array";
 
-    options.forEach((arg) => args.push(arg));
+    args.push(...options);
   }
 
-  return execAsync(`lp ${args.join(" ")}`);
+  return execFileAsync("lp", args);
 }

src/utils/exec-async.ts

@@ -1,25 +1,37 @@
 "use strict";
 
 import { ExecResponse } from "../types";
-import { exec } from "child_process";
+import { execFile } from "child_process";
 
-export default function execAsync(cmd: string): Promise<ExecResponse> {
-    return new Promise((resolve, reject) => {
-        exec(cmd, {
-            // The output from lp and lpstat is parsed assuming the language is English.
-            // LANG=C sets the language and the SOFTWARE variable is necessary
-            // on MacOS due to a detail in Apple's CUPS implementation
-            // (see https://unix.stackexchange.com/a/33836)
-            env: {
-                SOFTWARE: "",
-                LANG: "C"
-            }
-        }, (err, stdout, stderr) => {
-            if (err) {
-                reject(err);
-            } else {
-                resolve({stdout, stderr});
-            }
-        });
-    });
+export function execFileAsync(
+  cmd: string,
+  args: string[] = []
+): Promise<ExecResponse> {
+  return new Promise((resolve, reject) => {
+    execFile(
+      cmd,
+      args,
+      {
+        // The output from lp and lpstat is parsed assuming the language is English.
+        // LANG=C sets the language and the SOFTWARE variable is necessary
+        // on MacOS due to a detail in Apple's CUPS implementation
+        // (see https://unix.stackexchange.com/a/33836)
+        env: {
+          SOFTWARE: "",
+          LANG: "C",
+        },
+        // shell MUST be set to false.
+        // Otherwise any input containing shell metacharacters may be used to trigger arbitrary command execution.
+        // See https://nodejs.org/api/child_process.html#child_processexecfilefile-args-options-callback
+        shell: false,
+      },
+      (err, stdout, stderr) => {
+        if (err) {
+          reject(err);
+        } else {
+          resolve({ stdout, stderr });
+        }
+      }
+    );
+  });
 }