mirrors/electron
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions

fix: prevent node mode to be used as script runner by other apps (#40579)

Browse source
This commit is contained in:
Cheng Zhao 2023-12-06 11:23:54 +09:00 committed by GitHub
parent 9aa73abe78
commit cb0da6ff34
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 301 additions and 88 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
filenames.gni
View file

@ -199,6 +199,8 @@ filenames = {
"shell/common/language_util_mac.mm",
"shell/common/mac/main_application_bundle.h",
"shell/common/mac/main_application_bundle.mm",
"shell/common/mac/codesign_util.cc",
"shell/common/mac/codesign_util.h",
"shell/common/node_bindings_mac.cc",
"shell/common/node_bindings_mac.h",
"shell/common/platform_util_mac.mm",

33
shell/app/node_main.cc
View file

@ -13,6 +13,7 @@
#include "base/base_switches.h"
#include "base/command_line.h"
#include "base/containers/fixed_flat_set.h"
#include "base/environment.h"
#include "base/feature_list.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
@ -20,6 +21,7 @@
#include "base/task/thread_pool/thread_pool_instance.h"
#include "content/public/common/content_switches.h"
#include "electron/electron_version.h"
#include "electron/fuses.h"
#include "gin/array_buffer.h"
#include "gin/public/isolate_holder.h"
#include "gin/v8_initializer.h"
@ -35,13 +37,16 @@
#endif
#if BUILDFLAG(IS_LINUX)
#include "base/environment.h"
#include "base/posix/global_descriptors.h"
#include "base/strings/string_number_conversions.h"
#include "components/crash/core/app/crash_switches.h" // nogncheck
#include "content/public/common/content_descriptors.h"
#endif
#if BUILDFLAG(IS_MAC)
#include "shell/common/mac/codesign_util.h"
#endif
#if !IS_MAS_BUILD()
#include "components/crash/core/app/crashpad.h" // nogncheck
#include "shell/app/electron_crash_reporter_client.h"
@ -100,12 +105,36 @@ int NodeMain(int argc, char* argv[]) {
exit(1);
}
auto os_env = base::Environment::Create();
bool node_options_enabled = electron::fuses::IsNodeOptionsEnabled();
#if BUILDFLAG(IS_MAC)
if (node_options_enabled && os_env->HasVar("NODE_OPTIONS")) {
// On macOS, it is forbidden to run sandboxed app with custom arguments
// from another app, i.e. args are discarded in following call:
// exec("Sandboxed.app", ["--custom-args-will-be-discarded"])
// However it is possible to bypass the restriction by abusing the node mode
// of Electron apps:
// exec("Electron.app", {env: {ELECTRON_RUN_AS_NODE: "1",
// NODE_OPTIONS: "--require 'bad.js'"}})
// To prevent Electron apps from being used to work around macOS security
// restrictions, when NODE_OPTIONS is passed it will be checked whether
// this process is invoked by its own app.
if (!ProcessBelongToCurrentApp(getppid())) {
LOG(ERROR) << "NODE_OPTIONS is disabled because this process is invoked "
"by other apps.";
node_options_enabled = false;
}
}
#endif // BUILDFLAG(IS_MAC)
if (!node_options_enabled) {
os_env->UnSetVar("NODE_OPTIONS");
}
#if BUILDFLAG(IS_WIN)
v8_crashpad_support::SetUp();
#endif
#if BUILDFLAG(IS_LINUX)
auto os_env = base::Environment::Create();
std::string fd_string, pid_string;
if (os_env->GetVar("CRASHDUMP_SIGNAL_FD", &fd_string) &&
os_env->GetVar("CRASHPAD_HANDLER_PID", &pid_string)) {

55
shell/common/mac/codesign_util.cc Normal file
View file

@ -0,0 +1,55 @@
// Copyright 2023 Microsoft, Inc.
// Copyright 2013 The Chromium Authors
// Use of this source code is governed by the MIT license that can be
// found in the LICENSE file.
#include "shell/common/mac/codesign_util.h"
#include "base/apple/osstatus_logging.h"
#include "base/apple/scoped_cftyperef.h"
#include <CoreFoundation/CoreFoundation.h>
#include <Security/Security.h>
namespace electron {
bool ProcessBelongToCurrentApp(pid_t pid) {
// Get and check the code signature of current app.
base::apple::ScopedCFTypeRef<SecCodeRef> self_code;
OSStatus status =
SecCodeCopySelf(kSecCSDefaultFlags, self_code.InitializeInto());
if (status != errSecSuccess) {
OSSTATUS_LOG(ERROR, status) << "SecCodeCopyGuestWithAttributes";
return false;
}
// Get the code signature of process.
base::apple::ScopedCFTypeRef<CFNumberRef> process_cf(
CFNumberCreate(nullptr, kCFNumberIntType, &pid));
const void* attribute_keys[] = {kSecGuestAttributePid};
const void* attribute_values[] = {process_cf.get()};
base::apple::ScopedCFTypeRef<CFDictionaryRef> attributes(CFDictionaryCreate(
nullptr, attribute_keys, attribute_values, std::size(attribute_keys),
&kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks));
base::apple::ScopedCFTypeRef<SecCodeRef> process_code;
status = SecCodeCopyGuestWithAttributes(nullptr, attributes.get(),
kSecCSDefaultFlags,
process_code.InitializeInto());
if (status != errSecSuccess) {
OSSTATUS_LOG(ERROR, status) << "SecCodeCopyGuestWithAttributes";
return false;
}
// Get the requirement of current app's code signature.
base::apple::ScopedCFTypeRef<SecRequirementRef> self_requirement;
status = SecCodeCopyDesignatedRequirement(self_code.get(), kSecCSDefaultFlags,
self_requirement.InitializeInto());
if (status != errSecSuccess) {
OSSTATUS_LOG(ERROR, status) << "SecCodeCopyDesignatedRequirement";
return false;
}
// Check whether the process meets the signature requirement of current app.
status = SecCodeCheckValidity(process_code.get(), kSecCSDefaultFlags,
self_requirement.get());
return status == errSecSuccess;
}
} // namespace electron

19
shell/common/mac/codesign_util.h Normal file
View file

@ -0,0 +1,19 @@
// Copyright 2023 Microsoft, Inc.
// Copyright 2013 The Chromium Authors
// Use of this source code is governed by the MIT license that can be
// found in the LICENSE file.
#ifndef SHELL_COMMON_MAC_CODESIGN_UTIL_H_
#define SHELL_COMMON_MAC_CODESIGN_UTIL_H_
#include <unistd.h>
namespace electron {
// Given a pid, check if the process belongs to current app by comparing its
// code signature with current app.
bool ProcessBelongToCurrentApp(pid_t pid);
} // namespace electron
#endif // SHELL_COMMON_MAC_CODESIGN_UTIL_H_

96
spec/api-autoupdater-darwin-spec.ts
View file

@ -3,36 +3,26 @@ import * as cp from 'node:child_process';
import * as http from 'node:http';
import * as express from 'express';
import * as fs from 'fs-extra';
import * as os from 'node:os';
import * as path from 'node:path';
import * as psList from 'ps-list';
import { AddressInfo } from 'node:net';
import { ifdescribe, ifit } from './lib/spec-helpers';
import { copyApp, getCodesignIdentity, shouldRunCodesignTests, signApp, spawn, withTempDirectory } from './lib/codesign-helpers';
import * as uuid from 'uuid';
import { autoUpdater, systemPreferences } from 'electron';
const features = process._linkedBinding('electron_common_features');
const fixturesPath = path.resolve(__dirname, 'fixtures');
// We can only test the auto updater on darwin non-component builds
ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch === 'arm64') && !process.mas && !features.isComponentBuild())('autoUpdater behavior', function () {
ifdescribe(shouldRunCodesignTests)('autoUpdater behavior', function () {
this.timeout(120000);
let identity = '';
beforeEach(function () {
const result = cp.spawnSync(path.resolve(__dirname, '../script/codesign/get-trusted-identity.sh'));
if (result.status !== 0 || result.stdout.toString().trim().length === 0) {
// Per https://circleci.com/docs/2.0/env-vars:
// CIRCLE_PR_NUMBER is only present on forked PRs
if (process.env.CI && !process.env.CIRCLE_PR_NUMBER) {
throw new Error('No valid signing identity available to run autoUpdater specs');
}
const result = getCodesignIdentity();
if (result === null) {
this.skip();
} else {
identity = result.stdout.toString().trim();
identity = result;
}
});
@ -40,59 +30,6 @@ ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch ===
expect(identity).to.be.a('string').with.lengthOf.at.least(1);
});
const copyApp = async (newDir: string, fixture = 'initial') => {
const appBundlePath = path.resolve(process.execPath, '../../..');
const newPath = path.resolve(newDir, 'Electron.app');
cp.spawnSync('cp', ['-R', appBundlePath, path.dirname(newPath)]);
const appDir = path.resolve(newPath, 'Contents/Resources/app');
await fs.mkdirp(appDir);
await fs.copy(path.resolve(fixturesPath, 'auto-update', fixture), appDir);
const plistPath = path.resolve(newPath, 'Contents', 'Info.plist');
await fs.writeFile(
plistPath,
(await fs.readFile(plistPath, 'utf8')).replace('<key>BuildMachineOSBuild</key>', `<key>NSAppTransportSecurity</key>
<dict>
<key>NSAllowsArbitraryLoads</key>
<true/>
<key>NSExceptionDomains</key>
<dict>
<key>localhost</key>
<dict>
<key>NSExceptionAllowsInsecureHTTPLoads</key>
<true/>
<key>NSIncludesSubdomains</key>
<true/>
</dict>
</dict>
</dict><key>BuildMachineOSBuild</key>`)
);
return newPath;
};
const spawn = (cmd: string, args: string[], opts: any = {}) => {
let out = '';
const child = cp.spawn(cmd, args, opts);
child.stdout.on('data', (chunk: Buffer) => {
out += chunk.toString();
});
child.stderr.on('data', (chunk: Buffer) => {
out += chunk.toString();
});
return new Promise<{ code: number, out: string }>((resolve) => {
child.on('exit', (code, signal) => {
expect(signal).to.equal(null);
resolve({
code: code!,
out
});
});
});
};
const signApp = (appPath: string) => {
return spawn('codesign', ['-s', identity, '--deep', '--force', appPath]);
};
const launchApp = (appPath: string, args: string[] = []) => {
return spawn(path.resolve(appPath, 'Contents/MacOS/Electron'), args);
};
@ -107,17 +44,6 @@ ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch ===
return activeShipIts;
};
const withTempDirectory = async (fn: (dir: string) => Promise<void>, autoCleanUp = true) => {
const dir = await fs.mkdtemp(path.resolve(os.tmpdir(), 'electron-update-spec-'));
try {
await fn(dir);
} finally {
if (autoCleanUp) {
cp.spawnSync('rm', ['-r', dir]);
}
}
};
const logOnError = (what: any, fn: () => void) => {
try {
fn();
@ -151,7 +77,7 @@ ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch ===
(await fs.readFile(infoPath, 'utf8')).replace(/(<key>CFBundleShortVersionString<\/key>\s+<string>)[^<]+/g, `$1${version}`)
);
await mutateAppPreSign?.mutate(secondAppPath);
await signApp(secondAppPath);
await signApp(secondAppPath, identity);
await mutateAppPostSign?.mutate(secondAppPath);
updateZipPath = path.resolve(dir, 'update.zip');
await spawn('zip', ['-0', '-r', '--symlinks', updateZipPath, './'], {
@ -183,7 +109,7 @@ ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch ===
it('should cleanly set the feed URL when the app is signed', async () => {
await withTempDirectory(async (dir) => {
const appPath = await copyApp(dir);
await signApp(appPath);
await signApp(appPath, identity);
const launchResult = await launchApp(appPath, ['http://myupdate']);
expect(launchResult.code).to.equal(0);
expect(launchResult.out).to.include('Feed URL Set: http://myupdate');
@ -224,7 +150,7 @@ ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch ===
it('should hit the update endpoint when checkForUpdates is called', async () => {
await withTempDirectory(async (dir) => {
const appPath = await copyApp(dir, 'check');
await signApp(appPath);
await signApp(appPath, identity);
server.get('/update-check', (req, res) => {
res.status(204).send();
});
@ -241,7 +167,7 @@ ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch ===
it('should hit the update endpoint with customer headers when checkForUpdates is called', async () => {
await withTempDirectory(async (dir) => {
const appPath = await copyApp(dir, 'check-with-headers');
await signApp(appPath);
await signApp(appPath, identity);
server.get('/update-check', (req, res) => {
res.status(204).send();
});
@ -258,7 +184,7 @@ ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch ===
it('should hit the download endpoint when an update is available and error if the file is bad', async () => {
await withTempDirectory(async (dir) => {
const appPath = await copyApp(dir, 'update');
await signApp(appPath);
await signApp(appPath, identity);
server.get('/update-file', (req, res) => {
res.status(500).send('This is not a file');
});
@ -298,7 +224,7 @@ ifdescribe(process.platform === 'darwin' && !(process.env.CI && process.arch ===
infoPath,
(await fs.readFile(infoPath, 'utf8')).replace(/(<key>CFBundleShortVersionString<\/key>\s+<string>)[^<]+/g, '$11.0.0')
);
await signApp(appPath);
await signApp(appPath, identity);
const updateZipPath = await getOrCreateUpdateZipPath(opts.nextVersion, opts.endFixture, opts.mutateAppPreSign, opts.mutateAppPostSign);

22
spec/fixtures/api/fork-with-node-options.js vendored Normal file
View file

@ -0,0 +1,22 @@
const { execFileSync } = require('node:child_process');
const path = require('node:path');
const fixtures = path.resolve(__dirname, '..');
const env = {
ELECTRON_RUN_AS_NODE: 'true',
// Process will exit with 1 if NODE_OPTIONS is accepted.
NODE_OPTIONS: `--require "${path.join(fixtures, 'module', 'fail.js')}"`
};
// Provide a lower cased NODE_OPTIONS in case some code ignores case sensitivity
// when reading NODE_OPTIONS.
env.node_options = env.NODE_OPTIONS;
try {
execFileSync(process.argv[2],
['--require', path.join(fixtures, 'module', 'noop.js')],
{ env, stdio: 'inherit' });
process.exit(0);
} catch (error) {
console.log('NODE_OPTIONS passed to child');
process.exit(1);
}

99
spec/lib/codesign-helpers.ts Normal file
View file

@ -0,0 +1,99 @@
import * as cp from 'node:child_process';
import * as fs from 'fs-extra';
import * as os from 'node:os';
import * as path from 'node:path';
import { expect } from 'chai';
const features = process._linkedBinding('electron_common_features');
const fixturesPath = path.resolve(__dirname, '..', 'fixtures');
export const shouldRunCodesignTests =
process.platform === 'darwin' &&
!(process.env.CI && process.arch === 'arm64') &&
!process.mas &&
!features.isComponentBuild();
let identity: string | null;
export function getCodesignIdentity () {
if (identity === undefined) {
const result = cp.spawnSync(path.resolve(__dirname, '../../script/codesign/get-trusted-identity.sh'));
if (result.status !== 0 || result.stdout.toString().trim().length === 0) {
// Per https://circleci.com/docs/2.0/env-vars:
// CIRCLE_PR_NUMBER is only present on forked PRs
if (process.env.CI && !process.env.CIRCLE_PR_NUMBER) {
throw new Error('No valid signing identity available to run autoUpdater specs');
}
identity = null;
} else {
identity = result.stdout.toString().trim();
}
}
return identity;
}
export async function copyApp (newDir: string, fixture: string | null = 'initial') {
const appBundlePath = path.resolve(process.execPath, '../../..');
const newPath = path.resolve(newDir, 'Electron.app');
cp.spawnSync('cp', ['-R', appBundlePath, path.dirname(newPath)]);
if (fixture) {
const appDir = path.resolve(newPath, 'Contents/Resources/app');
await fs.mkdirp(appDir);
await fs.copy(path.resolve(fixturesPath, 'auto-update', fixture), appDir);
}
const plistPath = path.resolve(newPath, 'Contents', 'Info.plist');
await fs.writeFile(
plistPath,
(await fs.readFile(plistPath, 'utf8')).replace('<key>BuildMachineOSBuild</key>', `<key>NSAppTransportSecurity</key>
<dict>
<key>NSAllowsArbitraryLoads</key>
<true/>
<key>NSExceptionDomains</key>
<dict>
<key>localhost</key>
<dict>
<key>NSExceptionAllowsInsecureHTTPLoads</key>
<true/>
<key>NSIncludesSubdomains</key>
<true/>
</dict>
</dict>
</dict><key>BuildMachineOSBuild</key>`)
);
return newPath;
};
export function spawn (cmd: string, args: string[], opts: any = {}) {
let out = '';
const child = cp.spawn(cmd, args, opts);
child.stdout.on('data', (chunk: Buffer) => {
out += chunk.toString();
});
child.stderr.on('data', (chunk: Buffer) => {
out += chunk.toString();
});
return new Promise<{ code: number, out: string }>((resolve) => {
child.on('exit', (code, signal) => {
expect(signal).to.equal(null);
resolve({
code: code!,
out
});
});
});
};
export function signApp (appPath: string, identity: string) {
return spawn('codesign', ['-s', identity, '--deep', '--force', appPath]);
};
export async function withTempDirectory (fn: (dir: string) => Promise<void>, autoCleanUp = true) {
const dir = await fs.mkdtemp(path.resolve(os.tmpdir(), 'electron-update-spec-'));
try {
await fn(dir);
} finally {
if (autoCleanUp) {
cp.spawnSync('rm', ['-r', dir]);
}
}
};

63
spec/node-spec.ts
View file

@ -1,9 +1,10 @@
import { expect } from 'chai';
import * as childProcess from 'node:child_process';
import * as fs from 'node:fs';
import * as fs from 'fs-extra';
import * as path from 'node:path';
import * as util from 'node:util';
import { getRemoteContext, ifdescribe, ifit, itremote, useRemoteContext } from './lib/spec-helpers';
import { copyApp, getCodesignIdentity, shouldRunCodesignTests, signApp, spawn, withTempDirectory } from './lib/codesign-helpers';
import { webContents } from 'electron/main';
import { EventEmitter } from 'node:stream';
import { once } from 'node:events';
@ -659,6 +660,66 @@ describe('node feature', () => {
});
});
ifdescribe(shouldRunCodesignTests)('NODE_OPTIONS in signed app', function () {
let identity = '';
beforeEach(function () {
const result = getCodesignIdentity();
if (result === null) {
this.skip();
} else {
identity = result;
}
});
const script = path.join(fixtures, 'api', 'fork-with-node-options.js');
const nodeOptionsWarning = 'NODE_OPTIONS is disabled because this process is invoked by other apps';
it('is disabled when invoked by other apps in ELECTRON_RUN_AS_NODE mode', async () => {
await withTempDirectory(async (dir) => {
const appPath = await copyApp(dir);
await signApp(appPath, identity);
// Invoke Electron by using the system node binary as middle layer, so
// the check of NODE_OPTIONS will think the process is started by other
// apps.
const { code, out } = await spawn('node', [script, path.join(appPath, 'Contents/MacOS/Electron')]);
expect(code).to.equal(0);
expect(out).to.include(nodeOptionsWarning);
});
});
it('is disabled when invoked by alien binary in app bundle in ELECTRON_RUN_AS_NODE mode', async function () {
await withTempDirectory(async (dir) => {
const appPath = await copyApp(dir);
await signApp(appPath, identity);
// Find system node and copy it to app bundle.
const nodePath = process.env.PATH?.split(path.delimiter).find(dir => fs.existsSync(path.join(dir, 'node')));
if (!nodePath) {
this.skip();
return;
}
const alienBinary = path.join(appPath, 'Contents/MacOS/node');
await fs.copy(path.join(nodePath, 'node'), alienBinary);
// Try to execute electron app from the alien node in app bundle.
const { code, out } = await spawn(alienBinary, [script, path.join(appPath, 'Contents/MacOS/Electron')]);
expect(code).to.equal(0);
expect(out).to.include(nodeOptionsWarning);
});
});
it('is respected when invoked from self', async () => {
await withTempDirectory(async (dir) => {
const appPath = await copyApp(dir, null);
await signApp(appPath, identity);
const appExePath = path.join(appPath, 'Contents/MacOS/Electron');
const { code, out } = await spawn(appExePath, [script, appExePath]);
expect(code).to.equal(1);
expect(out).to.not.include(nodeOptionsWarning);
expect(out).to.include('NODE_OPTIONS passed to child');
});
});
});
describe('Node.js cli flags', () => {
let child: childProcess.ChildProcessWithoutNullStreams;
let exitPromise: Promise<any[]>;