doyensec / electronegativity

Electronegativity is a tool to identify misconfigurations and security anti-patterns in Electron applications.
Apache License 2.0
972 stars 66 forks source link

Cannot manually whitelist PERMISSION_REQUEST_HANDLER_GLOBAL_CHECK if using a partition #84

Closed reZach closed 3 years ago

reZach commented 3 years ago

Describe the bug When using a session partition, one cannot exclude the check for PERMISSION_REQUEST_HANDLER_GLOBAL_CHECK.

To Reproduce Notice on line 50 (ses.fromPartition(partition)), one cannot exclude from the tool from throwing an audit error.

const {
    app,
    BrowserWindow,
    session
} = require("electron");
const path = require("path");
const isDev = process.env.NODE_ENV === "development";
const port = 40992; // Hardcoded; needs to match webpack.development.js and package.json
const selfHost = `http://localhost:${port}`;

// Keep a global reference of the window object, if you don't, the window will
// be closed automatically when the JavaScript object is garbage collected.
let win;

async function createWindow() {

    // Create the browser window.
    win = new BrowserWindow({
        width: 800,
        height: 600,
        title: "Application is currently initializing...",
        webPreferences: {
            devTools: isDev,
            nodeIntegration: false,
            nodeIntegrationInWorker: false,
            nodeIntegrationInSubFrames: false,
            contextIsolation: true,
            enableRemoteModule: false,
            additionalArguments: [`storePath:${app.getPath("userData")}`],
            preload: path.join(__dirname, "preload.js"),
            /* eng-disable PRELOAD_JS_CHECK */
            disableBlinkFeatures: "Auxclick"
        }
    });

    // Load app
    win.loadURL(selfHost);

    // Emitted when the window is closed.
    win.on("closed", () => {
        // Dereference the window object, usually you would store windows
        // in an array if your app supports multi windows, this is the time
        // when you should delete the corresponding element.
        win = null;
    });

    // https://electronjs.org/docs/tutorial/security#4-handle-session-permission-requests-from-remote-content
    const ses = session;
    const partition = "default";
    ses.fromPartition(partition) /* eng-disable PERMISSION_REQUEST_HANDLER_JS_CHECK */
        .setPermissionRequestHandler((webContents, permission, permCallback) => {
            let allowedPermissions = []; // Full list here: https://developer.chrome.com/extensions/declare_permissions#manifest

            if (allowedPermissions.includes(permission)) {
                permCallback(true); // Approve permission request
            } else {
                console.error(
                    `The application tried to request permission for '${permission}'. This permission was not whitelisted and has been blocked.`
                );

                permCallback(false); // Deny
            }
        });
}

// This method will be called when Electron has finished
// initialization and is ready to create browser windows.
// Some APIs can only be used after this event occurs.
app.on("ready", createWindow);

The error thrown in the tool is this

│ PERMISSION_REQUEST_HANDLER_GLOBAL_CHECK │ N/A                                                                         │ 
0:0      │ Missing PermissionRequestHandler to limit        │
│ MEDIUM | CERTAIN                        │                                                                             │ 
         │ specific permissions (e.g. openExternal) in      │
│                                         │                                                                             │ 
         │ response to events from particular origins.      │
│                                         │                                                                             │ 
         │ https://git.io/JeuM0                             │

Expected behavior /* eng-disable PERMISSION_REQUEST_HANDLER_JS_CHECK */ should work if using a partition.

Stacktraces None.

Platform (please complete the following information):

Additional context For use in secure-electron-template, I'd like to use your tool to validate my template is secure 😄.

phosphore commented 3 years ago

See https://github.com/doyensec/electronegativity/issues/85#issuecomment-777369529, you would need to use the -x flag to exclude specific checks (Global||Atomic) in these few cases where a finding is issues nonetheless (something is missing vs something important marked for manual review anyway).

reZach commented 3 years ago

See #85 (comment), you would need to use the -x flag to exclude specific checks (Global||Atomic) in these few cases where a finding is issues nonetheless (something is missing vs something important marked for manual review anyway).

Thanks for your help!