ELECTRON: security fix (#319)

electron/fileError.txt

@@ -0,0 +1 @@
+Attempts to access local files outside the normal game environment will be directed to this file.

electron/gameWindow.js

@@ -7,9 +7,7 @@ const menu = require("./menu");
 const api = require("./api-server");
 const cp = require("child_process");
 const path = require("path");
-const fs = require("fs");
 const { windowTracker } = require("./windowTracker");
-const { fileURLToPath } = require("url");
 
 const debug = process.argv.includes("--debug");
 
@@ -62,30 +60,8 @@ async function createWindow(killall) {
       return;
     }
 
-    // make sure local urls stay in electron perimeter
+    // Just use the default handling for file requests, they should be intercepted in main.js file protocol intercept.
-    if (url.substr(0, "file://".length) === "file://") {
+    if (url.startswith("file://")) return;
-      const requestedPath = fileURLToPath(url);
-      const appPath = path.parse(app.getAppPath());
-      const filePath = path.parse(requestedPath);
-      const isChild = filePath.dir.startsWith(appPath.dir);
-
-      // eslint-disable-next-line no-sync
-      const fileExists = fs.existsSync(requestedPath);
-
-      if (!isChild) {
-        // If we're not relative to our app's path let's abort
-        log.warn(
-          `Requested path ${filePath.dir}${path.sep}${filePath.base} is not relative to the app: ${appPath.dir}${path.sep}${appPath.base}`,
-        );
-        e.preventDefault();
-      } else if (!fileExists) {
-        // If the file does not exist let's abort
-        log.warn(`Requested path ${filePath.dir}${path.sep}${filePath.base} does not exist`);
-        e.preventDefault();
-      }
-
-      return;
-    }
 
     if (process.platform === "win32") {
       // If we have parameters in the URL, explorer.exe won't register the URL and will open the file explorer instead.
@@ -119,7 +95,7 @@ async function createWindow(killall) {
   }
 
   menu.refreshMenu(window);
-  setStopProcessHandler(app, window, true);
+  setStopProcessHandler(app, window);
 
   return window;
 }

electron/main.js

@@ -1,6 +1,6 @@
 /* eslint-disable no-process-exit */
 /* eslint-disable @typescript-eslint/no-var-requires */
-const { app, dialog, BrowserWindow, ipcMain } = require("electron");
+const { app, dialog, BrowserWindow, ipcMain, protocol } = require("electron");
 const log = require("electron-log");
 const greenworks = require("./greenworks");
 const api = require("./api-server");
@@ -11,6 +11,8 @@ const storage = require("./storage");
 const debounce = require("lodash/debounce");
 const Config = require("electron-config");
 const config = new Config();
+const path = require("path");
+const { fileURLToPath } = require("url");
 
 log.transports.file.level = config.get("file-log-level", "info");
 log.transports.console.level = config.get("console-log-level", "debug");
@@ -20,6 +22,7 @@ log.info(`Started app: ${JSON.stringify(process.argv)}`);
 
 process.on("uncaughtException", function () {
   // The exception will already have been logged by electron-log
+  app.quit();
   process.exit(1);
 });
 
@@ -39,7 +42,14 @@ try {
 
 let isRestoreDisabled = false;
 
-function setStopProcessHandler(app, window, enabled) {
+// This was moved so that startup errors do not lead to ghost processes
+app.on("window-all-closed", () => {
+  log.info("Quitting the app...");
+  app.quit();
+  process.exit(0);
+});
+
+function setStopProcessHandler(app, window) {
   const closingWindowHandler = async (e) => {
     // We need to prevent the default closing event to add custom logic
     e.preventDefault();
@@ -102,18 +112,8 @@ function setStopProcessHandler(app, window, enabled) {
     window = null;
   };
 
-  const stopProcessHandler = () => {
-    log.info("Quitting the app...");
-    app.isQuiting = true;
-    app.quit();
-    process.exit(0);
-  };
-
   const receivedGameReadyHandler = async (event, arg) => {
-    if (!window) {
+    if (!window) return log.warn("Window was undefined in game info handler");
-      log.warn("Window was undefined in game info handler");
-      return;
-    }
 
     log.debug("Received game information", arg);
     window.gameInfo = { ...arg };
@@ -130,10 +130,7 @@ function setStopProcessHandler(app, window, enabled) {
   };
 
   const receivedDisableRestoreHandler = async (event, arg) => {
-    if (!window) {
+    if (!window) return log.warn("Window was undefined in disable import handler");
-      log.warn("Window was undefined in disable import handler");
-      return;
-    }
 
     log.debug(`Disabling auto-restore for ${arg.duration}ms.`);
     isRestoreDisabled = true;
@@ -144,10 +141,7 @@ function setStopProcessHandler(app, window, enabled) {
   };
 
   const receivedGameSavedHandler = async (event, arg) => {
-    if (!window) {
+    if (!window) return log.warn("Window was undefined in game saved handler");
-      log.warn("Window was undefined in game saved handler");
-      return;
-    }
 
     const { save, ...other } = arg;
     log.silly("Received game saved info", { ...other, save: `${save.length} bytes` });
@@ -198,21 +192,12 @@ function setStopProcessHandler(app, window, enabled) {
     { leading: true },
   );
 
-  if (enabled) {
+  log.debug("Adding closing handlers");
-    log.debug("Adding closing handlers");
+  ipcMain.on("push-game-ready", receivedGameReadyHandler);
-    ipcMain.on("push-game-ready", receivedGameReadyHandler);
+  ipcMain.on("push-game-saved", receivedGameSavedHandler);
-    ipcMain.on("push-game-saved", receivedGameSavedHandler);
+  ipcMain.on("push-disable-restore", receivedDisableRestoreHandler);
-    ipcMain.on("push-disable-restore", receivedDisableRestoreHandler);
+  window.on("closed", clearWindowHandler);
-    window.on("closed", clearWindowHandler);
+  window.on("close", closingWindowHandler);
-    window.on("close", closingWindowHandler);
-    app.on("window-all-closed", stopProcessHandler);
-  } else {
-    log.debug("Removing closing handlers");
-    ipcMain.removeAllListeners();
-    window.removeListener("closed", clearWindowHandler);
-    window.removeListener("close", closingWindowHandler);
-    app.removeListener("window-all-closed", stopProcessHandler);
-  }
 }
 
 async function startWindow(noScript) {
@@ -221,17 +206,27 @@ async function startWindow(noScript) {
 
 global.app_handlers = {
   stopProcess: setStopProcessHandler,
-  createWindow: startWindow,
 };
 
-app.whenReady().then(async () => {
+app.on("ready", async () => {
-  log.info("Application is ready!");
+  // Intercept file protocol requests and only let valid requests through
+  protocol.interceptFileProtocol("file", ({ url, method }, callback) => {
+    const filePath = fileURLToPath(url);
+    const relativePath = path.relative(__dirname, filePath);
+    //only provide html files in same directory, or anything in dist
+    if ((method === "GET" && relativePath.startsWith("dist")) || relativePath.match(/^[a-zA-Z-_]*\.html/)) {
+      return callback(filePath);
+    }
+    log.error("Tried to access a page outside sandbox.");
+    callback(path.join(__dirname, "fileError.txt"));
+  });
 
+  log.info("Application is ready!");
   if (process.argv.includes("--export-save")) {
     const window = new BrowserWindow({ show: false });
-    await window.loadFile("export.html", false);
+    await window.loadFile("export.html");
     window.show();
-    setStopProcessHandler(app, window, true);
+    setStopProcessHandler(app, window);
     await utils.exportSave(window);
   } else {
     const window = await startWindow(process.argv.includes("--no-scripts"));

electron/utils.js

@@ -1,31 +1,14 @@
 /* eslint-disable @typescript-eslint/no-var-requires */
-const { app, dialog, shell } = require("electron");
+const { dialog, shell } = require("electron");
 const log = require("electron-log");
 
-const achievements = require("./achievements");
-const api = require("./api-server");
-
 const Config = require("electron-config");
 const config = new Config();
 
 function reloadAndKill(window, killScripts) {
-  const setStopProcessHandler = global.app_handlers.stopProcess;
-  const createWindowHandler = global.app_handlers.createWindow;
-
   log.info("Reloading & Killing all scripts...");
-  setStopProcessHandler(app, window, false);
-
-  achievements.disableAchievementsInterval(window);
-  api.disable();
-
   window.webContents.forcefullyCrashRenderer();
-  window.on("closed", () => {
+  window.loadFile("index.html", killScripts ? { query: { noScripts: true } } : {});
-    // Wait for window to be closed before opening the new one to prevent race conditions
-    log.debug("Opening new window");
-    createWindowHandler(killScripts);
-  });
-
-  window.close();
 }
 
 function promptForReload(window) {