Arbitrary filesystem manipulation vulnerability introduced by IPC exposure #2143

xiaofen9 · 2021-03-09T16:17:42Z

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(92ba66d), which is good. However, such direct IPC export may re-expose many sensitive primitives to the attacker.

Here is the exposure site.

camunda-modeler/app/lib/preload.js

Lines 27 to 37 in a587434

    
           window.getAppPreload = function() { 
        
             return { 
        
               metadata: app.metadata, 
        
               plugins: app.plugins.getAll(), 
        
               flags: app.flags.getAll(), 
        
               ipcRenderer, 
        
               platform 
        
             }; 
        
           };

By sending a message to file:write channel. The attacker may read and write malicious content to the user filesystem.

camunda-modeler/app/lib/index.js

Lines 186 to 194 in 6f1497c

    
           renderer.on('file:write', function(filePath, file, options = {}, done) { 
        
             try { 
        
               const newFile = writeFile(filePath, file, options); 
        
               done(null, newFile); 
        
             } catch (err) { 
        
               done(err); 
        
             } 
        
           });

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

The text was updated successfully, but these errors were encountered:

nikku · 2021-03-11T16:19:00Z

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

nikku · 2021-03-11T16:37:52Z

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 · 2021-03-11T16:56:13Z

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 · 2021-03-12T07:44:19Z

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.

This prevents using `window.getAppPreload` by other code than the application. Thus, only the application has access to the IPCRenderer and can decide which APIs it provides. Closes #2143. Related to https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-28154

barmac · 2021-03-15T14:12:56Z

With #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.

xiaofen9 added the bug Something isn't working label Mar 9, 2021

xiaofen9 changed the title ~~Arbitrary filesystem manipulation vulnerability introduced by ipc exposure~~ Arbitrary filesystem manipulation vulnerability introduced by IPC exposure Mar 9, 2021

nikku added help wanted Extra attention is needed ready Ready to be worked on and removed ready Ready to be worked on labels Mar 12, 2021

bpmn-io-tasks bot assigned barmac Mar 15, 2021

bpmn-io-tasks bot added the in progress Currently worked on label Mar 15, 2021

barmac mentioned this issue Mar 15, 2021

fix(remote): remove getAppPreload once loaded #2155

Merged

bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Mar 15, 2021

barmac closed this as completed Mar 15, 2021

bpmn-io-tasks bot removed the needs review Review pending label Mar 15, 2021

barmac added the wontfix This will not be worked on label Mar 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Arbitrary filesystem manipulation vulnerability introduced by IPC exposure #2143

Arbitrary filesystem manipulation vulnerability introduced by IPC exposure #2143

xiaofen9 commented Mar 9, 2021 •

edited by nikku

nikku commented Mar 11, 2021

nikku commented Mar 11, 2021 •

edited by philippfromme

xiaofen9 commented Mar 11, 2021

nikku commented Mar 12, 2021 •

edited

barmac commented Mar 15, 2021 •

edited

Arbitrary filesystem manipulation vulnerability introduced by IPC exposure #2143

Arbitrary filesystem manipulation vulnerability introduced by IPC exposure #2143

Comments

xiaofen9 commented Mar 9, 2021 • edited by nikku

nikku commented Mar 11, 2021

nikku commented Mar 11, 2021 • edited by philippfromme

xiaofen9 commented Mar 11, 2021

nikku commented Mar 12, 2021 • edited

barmac commented Mar 15, 2021 • edited

xiaofen9 commented Mar 9, 2021 •

edited by nikku

nikku commented Mar 11, 2021 •

edited by philippfromme

nikku commented Mar 12, 2021 •

edited

barmac commented Mar 15, 2021 •

edited