refactor: ensure IpcRenderer is not bridgable by MarshallOfSound · Pull Request #40330 · electron/electron · GitHub
Skip to content

refactor: ensure IpcRenderer is not bridgable - #40330

Merged
jkleinsc merged 3 commits into
mainfrom
prevent-ipc-renderer-bridge
Oct 31, 2023
Merged

jkleinsc merged 3 commits into
mainfrom
prevent-ipc-renderer-bridge

Conversation

@MarshallOfSound

Copy link
Copy Markdown
Member

It's a security footgun that IpcRenderer (unlike everything else) can go over the context bridge by accident. Let's just make it so it can't

Notes: no-notes

@electron-cation electron-cation Bot added the new-pr 🌱 PR opened recently label Oct 25, 2023
@MarshallOfSound MarshallOfSound added semver/patch backwards-compatible bug fixes no-backport labels Oct 25, 2023
@miniak

miniak commented Oct 25, 2023

Copy link
Copy Markdown
Contributor

@MarshallOfSound

Copy link
Copy Markdown
Member Author

@miniak Oh yeah sure

@electron-cation electron-cation Bot removed the new-pr 🌱 PR opened recently label Oct 26, 2023
@dsanders11

Copy link
Copy Markdown
Member

Should this be considered a breaking change? In #40321 the docs are being updated to avoid the footgun pattern, but currently the IPC tutorial said "In the renderer process, use the event parameter to send a reply back to the main process [...]" (and gave a code example doing so). If someone followed that tutorial, won't this change break their code?

@MarshallOfSound

Copy link
Copy Markdown
Member Author

Should this be considered a breaking change?

In the sense we could note it somewhere, probably, in the sense that it's semver/major no. Security fixes aren't considered semver/major. No plans to backport this one though

@dsanders11

Copy link
Copy Markdown
Member

In the sense we could note it somewhere, probably

Let's add it to breaking changes then. 👍

@MarshallOfSound
MarshallOfSound force-pushed the prevent-ipc-renderer-bridge branch from 60ae5ba to d48bd5a Compare October 31, 2023 05:48
@MarshallOfSound
MarshallOfSound requested a review from a team as a code owner October 31, 2023 05:48

@jkleinsc jkleinsc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this change is causing the node feature does not hang when using the fs module in the renderer process test to fail.

@jkleinsc
jkleinsc merged commit 83892ab into main Oct 31, 2023
@jkleinsc
jkleinsc deleted the prevent-ipc-renderer-bridge branch October 31, 2023 21:29
@release-clerk

release-clerk Bot commented Oct 31, 2023

Copy link
Copy Markdown

MrHuangJser pushed a commit to MrHuangJser/electron that referenced this pull request Dec 11, 2023
* refactor: ensure IpcRenderer is not bridgable

* chore: add notes to breaking-changes

* spec: fix test that bridged ipcrenderer
@dsanders11 dsanders11 mentioned this pull request Aug 5, 2024
17 of 18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-backport semver/patch backwards-compatible bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants