Camunda-modeler: Arbitrary filesystem manipulation vulnerability introduced by IPC exposure

[xiaofen9]

__Describe the Bug__
Hi,

Great work!
We did a security analysis and found that app/lib/preload.js directly expose risky ipcRenderer instance to unsafe renderer process, which enables a remote attacker to abuse sensitive methods in the main process by crafting malicious ipc message.

I notice that the app has already disabled node integration in unsafe renderers(https://github.com/camunda/camunda-modeler/commit/92ba66d3e7ae85130b71cf7fc869b2066aadf67f), which is good. However, such direct IPC export may re-expose many sensitive primitives to the attacker.

Here is the exposure site.
https://github.com/camunda/camunda-modeler/blob/a5874345f259188b39bd6b2448a64e238fa700bb/app/lib/preload.js#L27-L37

By sending a message to file:write channel. The attacker may read and write malicious content to the user filesystem.
https://github.com/camunda/camunda-modeler/blob/6f1497c41917bc12bb2e44b3fa3c49b36f32ee72/app/lib/index.js#L186-L194

__Expected Behavior__
I could think of two possible solutions:

• enforce security checks when receiving events on sensitive channels (e.g., file read/write).
• avoid directly exposing ipcRenderer to untrusted domains.

---

Ref. CVE-2021-28154

[nikku]

Thanks for your heads up. We'll have a look.

[nikku]

To better understand the impact of what you've found, could you provide us a few more details on the following:

[...] which enables a remote attacker to abuse sensitive methods in the main process by crafting malicious ipc message.

The way we secured the app is that it does not allow any remote scripts to be opened, no unsafe scripts to be evaluated, no remote sites to be browsed. Could you elaborate how a _remote_ attacker is able to exploit that issue?

I could think of two possible solutions [...]

Regarding the solutions you mentioned, do you have any concrete examples how these mitigate the risk? Any patterns / best practices you have in mind?

[xiaofen9]

For the first question:
First of all, besides unsafe remote script, XSS may also be the entry point for the exploit. It is a good practice to have policies like not loading unsafe scripts, however, may we guarantee that there will always no such violations or no XSS in the future? It is usually better to enforce security by isolating/checking vulnerable APIs rather than relying on some assumptions, right?

For the second question:
During my analysis on other apps, I do notice more secure IPC implementations that follow the principle of least priviliege. For example, when preloading IPC to the unsafe renderer process, some apps choose to only export IPC channels required by a certain renderer rather than giving full IPC access.

[nikku]

We take our applications security seriously, follow Electron security best practices and carefully examine the impact of reported vulnerabilities. So thanks again for approaching us and getting back in a timely manner.

Your bug report explicitly states that arbitrary file system manipulation is possible by _remote_ attackers / when rendering _untrusted_ pages in the render process.

We validated our initial assessment and can confirm that this is not the case. As mentioned only trusted content is being loaded into the render process. Measures outlined by the Electron security best practices prevent any untrusted websites or scripts from being opened, included or accessed. Any break in that trust model (XSS, include of a remote resource) is a serious security thread that we will handle with care.

We will consider actions to further harden the security as you suggested. Our app is an editor for local files and accessing arbitrary files is a _feature_. We cannot get rid of file system access easily, unfortunately.

[barmac]

With https://github.com/camunda/camunda-modeler/pull/2155, we remove the access to window.getAppPreload as soon as the client part of the app is loaded. Still, we cannot remove access to the file system via ipcRenderer as it's in the essence of the local files editor.