Security breach

[brrd]

The nodeIntegration is enabled in Abricotine renderer process, which means that any JavaScript code executed in the editor has a potential access to your computer files and your system API.

This can be dangerous when previewing untrusted contents, such as iframes and images.

All version are concerned.

To reduce the risk you can :

• Disable iframe and images autopreview,
• Avoid editing untrusted documents,
• Use another markdow editor until this is fixed.

How to fix

Fulfill the prerequisites of https://electronjs.org/docs/tutorial/security

Especially:

Disable the Node.js integration in all renderers that display remote content

This probably implies a major refactoring of the code.

[bard]

I'm not familiar with the code base, just "driving by" while looking for a markdown editor for a friend, so pardon me if this is way off the mark, but couldn't external resources such as iframes and images just be wrapped in <webview>? From a little googling I see that iframes are not by sandboxed (at least by default) but webviews are.

[brrd]

@bard This is interesting. The idea would be to wrap previewed external resources into webviews with disabled nodeIntegration. This can make a good quick fix.

Anyway I keep thinking the safest solution would be to disable nodeIntegration for the whole renderer process, but we can do this later.

Thank you for this suggestion.

[bard]

Yes, that was just an idea in the spirit of "1) make it work, 2) make it right, 3) make it fast". Also I came across https://github.com/electron/electron/issues/200#issuecomment-36742564 which seems to say that usual same-origin policies apply, so malicious code in an external iframe shouldn't be able to reach to the outer (containing) window, but again I'm no expert with electron and that issue is quite old. Good luck in any case!

[brrd]

You are totally right, and by reading your comment I suddenly remembered this commit 754a6f0313d8ca3a36b28a4e00c9fc725b442365 pushed 3 years ago where I already wrapped iframe into webviews for security purpose. This was totally out of my mind :expressionless:

So the only thing to do now is to do the same with images in order to prevent attacks based on stegosploit.

In the end this issue was much less critical than I thought. Thanks again for pointing this out.

[bernardoadc]

The stegosploit website mentioned is far too long for me to read entirely, but it seems there is no solution yet. They actually claim "These payloads are undetectable using current means".

So if it is a broader security breach for all web, I don't think is something this project should be concerned specifically, right?

[brrd]

@bernardoadc In conventional browsers, JavaScript environment only has access to the browser's API, which can be considered safe. Such security breaches aren't a big threat in most situations.

In Electron, JavaScript can eventually access to lower level methods of the computer (such as read/write on the disk), which is in a bigger concern. As a solution, we are planning to encapsulate untrusted contents in webviews with a browser-level API.

I admit that there is very little chance that an Abricotine user will be targeted by stegosploit as it is not an easy attack to set up. But I want to make sure that users still get the best security.

[bernardoadc]

Sure, I agree that altough the breach surface is small, the damage potential is a big deal in electron-based apps. But that still applies to every electron app, right? IMHO this should be an issue in their repo, not this one.

[brrd]

But that still applies to every electron app, right?

@bernardoadc Not really. Electron can be safe. It depends on how you design the app and how you use Node integration. For further information you can read https://www.electronjs.org/docs/tutorial/security

[brrd]

After further investigation I came to the conclusion that the only way to perform stegosploit in Abricotine would be to serve images with a modified content-type. So in order to prevent this, unexpected content-types (eg. all except image/xxx) are now filtered out from received headers by Abricotine (905a05bda25f70b03a7a4aef3fd60d1db3e734c1). This is not the case in webviews though (which if safe because node integration is disabled in webviews), in order to allow previewing iframes.