Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: apply csp correctly when contextIsolation: false #37839

Merged
merged 1 commit into from
Apr 5, 2023
Merged

fix: apply csp correctly when contextIsolation: false #37839

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 19 additions & 2 deletions shell/common/node_bindings.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,10 @@ v8::ModifyCodeGenerationFromStringsResult ModifyCodeGenerationFromStrings(
v8::Local<v8::Context> context,
v8::Local<v8::Value> source,
bool is_code_like) {
// If we're running with contextIsolation enabled in the renderer process,
// fall back to Blink's logic.
if (node::Environment::GetCurrent(context) == nullptr) {
// No node environment means we're in the renderer process, either in a
// sandboxed renderer or in an unsandboxed renderer with context isolation
// enabled.
if (gin_helper::Locker::IsBrowserProcess()) {
NOTREACHED();
return {false, {}};
Expand All @@ -195,6 +196,22 @@ v8::ModifyCodeGenerationFromStringsResult ModifyCodeGenerationFromStrings(
context, source, is_code_like);
}

// If we get here then we have a node environment, so either a) we're in the
// main process, or b) we're in the renderer process in a context that has
// both node and blink, i.e. contextIsolation disabled.

// If we're in the main process, delegate to node.
if (gin_helper::Locker::IsBrowserProcess()) {
return node::ModifyCodeGenerationFromStrings(context, source, is_code_like);
}

// If we're in the renderer with contextIsolation disabled, ask blink first
// (for CSP), and iff that allows codegen, delegate to node.
v8::ModifyCodeGenerationFromStringsResult result =
blink::V8Initializer::CodeGenerationCheckCallbackInMainThread(
context, source, is_code_like);
if (!result.codegen_allowed)
return result;
return node::ModifyCodeGenerationFromStrings(context, source, is_code_like);
}

Expand Down
88 changes: 67 additions & 21 deletions spec/chromium-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,28 +355,74 @@ describe('web security', () => {
});
});

describe('csp in sandbox: false', () => {
it('is correctly applied', async () => {
const w = new BrowserWindow({
show: false,
webPreferences: { sandbox: false }
describe('csp', () => {
for (const sandbox of [true, false]) {
describe(`when sandbox: ${sandbox}`, () => {
for (const contextIsolation of [true, false]) {
describe(`when contextIsolation: ${contextIsolation}`, () => {
it('prevents eval from running in an inline script', async () => {
const w = new BrowserWindow({
show: false,
webPreferences: { sandbox, contextIsolation }
});
w.loadURL(`data:text/html,<head>
<meta http-equiv="Content-Security-Policy" content="default-src 'self'; script-src 'self' 'unsafe-inline'">
</head>
<script>
try {
// We use console.log here because it is easier than making a
// preload script, and the behavior under test changes when
// contextIsolation: false
console.log(eval('true'))
} catch (e) {
console.log(e.message)
}
</script>`);
const [,, message] = await emittedOnce(w.webContents, 'console-message');
expect(message).to.match(/Refused to evaluate a string/);
});

it('does not prevent eval from running in an inline script when there is no csp', async () => {
const w = new BrowserWindow({
show: false,
webPreferences: { sandbox, contextIsolation }
});
w.loadURL(`data:text/html,
<script>
try {
// We use console.log here because it is easier than making a
// preload script, and the behavior under test changes when
// contextIsolation: false
console.log(eval('true'))
} catch (e) {
console.log(e.message)
}
</script>`);
const [,, message] = await emittedOnce(w.webContents, 'console-message');
expect(message).to.equal('true');
});

it('prevents eval from running in executeJavaScript', async () => {
const w = new BrowserWindow({
show: false,
webPreferences: { sandbox, contextIsolation }
});
w.loadURL('data:text/html,<head><meta http-equiv="Content-Security-Policy" content="default-src \'self\'; script-src \'self\' \'unsafe-inline\'"></meta></head>');
await expect(w.webContents.executeJavaScript('eval("true")')).to.be.rejected();
});

it('does not prevent eval from running in executeJavaScript when there is no csp', async () => {
const w = new BrowserWindow({
show: false,
webPreferences: { sandbox, contextIsolation }
});
w.loadURL('data:text/html,');
expect(await w.webContents.executeJavaScript('eval("true")')).to.be.true();
});
});
}
});
w.loadURL(`data:text/html,<head>
<meta http-equiv="Content-Security-Policy" content="default-src 'self'; script-src 'self' 'unsafe-inline'">
</head>
<script>
try {
// We use console.log here because it is easier than making a
// preload script, and the behavior under test changes when
// contextIsolation: false
console.log(eval('failure'))
} catch (e) {
console.log('success')
}
</script>`);
const [,, message] = await emittedOnce(w.webContents, 'console-message');
expect(message).to.equal('success');
});
}
});

it('does not crash when multiple WebContent are created with web security disabled', () => {
Expand Down