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.

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

Desktop: Ignore no-sandbox start flag at joplin startup #2436

Merged
merged 1 commit into from
Feb 5, 2020

Conversation

vsimkus
Copy link
Contributor

@vsimkus vsimkus commented Feb 4, 2020

The change allows --no-sandbox flag at Joplin desktop app startup. This enables the users to optionally run the app without the electron sandbox as a workaround for the electron + AppImage package issue discussed in #2246.

@thearchivalone
Copy link
Contributor

What are the negative consequences of running with this flag enabled and used?

@vsimkus
Copy link
Contributor Author

vsimkus commented Feb 4, 2020

What are the negative consequences of running with this flag enabled and used?

The sandbox is essentially a security feature, so you would be disabling a security feature that is supposed to protect your system against exploits in the electron renderer. It is generally safe for desktop apps to run without the sandbox if they're not loading remote content. So for your daily note-taking this should be fine.

However, one possible exploit surface would be the webclipper. For example, if you used the webclipper "Clip complete page (HTML)" option on a website that contains malicious javascript, you could potentially end up running that malicious script in Joplin, which in turn might compromise your system. However, if you don't use the webclipper HTML option and you don't load remote content in your notes otherwise, then you should be safe.

@tessus
Copy link
Collaborator

tessus commented Feb 4, 2020

TL;TD

It's less secure. Somewhat ironic, isn't it?

Explanation

The reason to run an app in a sandbox is security. It then is encapsulated and can't break out of that sandbox. Imagine a chroot environment.
However, here's the rub: To make a sandbox truly secure (there are other ways too, but often error prone and easier to circumvent), the sandbox itself must be started with root, so that the user processes can't break out. At least that is the excuse for Electron requiring the sandbox to be run as root.
The strange part is that Debian (which is supposed to be security-sensitive) deactivated that feature in the kernel for security reason. Whaaaaat?!?!?

@vsimkus
Copy link
Contributor Author

vsimkus commented Feb 4, 2020

The strange part is that Debian (which is supposed to be security-sensitive) deactivated that feature in the kernel for security reason. Whaaaaat?!?!?

It's quite straightforward. Before running the binary the OS doesn't know if it is a sandbox initialiser. So the default is to disable such permission. Again, the issue is not in Debian, but electron.

Also, in the past it has been shown that it was possible to break out of the chromium-sandbox. In that case breaking out of the sandbox automatically gives you root permissions to all of the system. I think Debian developers made the right call disabling it. But this is off-topic.

@tessus tessus added the desktop All desktop platforms label Feb 4, 2020
@thearchivalone
Copy link
Contributor

thearchivalone commented Feb 4, 2020

Thanks for verifying my thoughts on it. Since ElectronBuild supports Debian builds, would building a debian package that installs and integrates directly into the system possibly bypass this issue without introducing the security hole you are both suggesting? I'm almost certain someone was able extract the appimage and able to run Joplin straight from the rootfs/bin folder with no issues.

Or am i misunderstanding how debian packages work? And, would an Electron Builder one be just a debian version of AppImage or what? And it would mainly be for those that are having this issue if it works and not the main way of running the app.

https://www.electron.build/configuration/linux

@laurent22
Copy link
Owner

Looks good, thanks @vsimkus! As for how the flag is used, it's up to the user.

@laurent22 laurent22 merged commit 69fc518 into laurent22:master Feb 5, 2020
@thearchivalone
Copy link
Contributor

Grats, @vsimkus . :D

@andrey-lazarev
Copy link

Debian 10 reporting: --no-sandbox options worked for me, thanks.

@error420
Copy link

error420 commented Mar 1, 2021

"./Joplin.AppImage --no-sandbox" works for me too! Debian 10.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
desktop All desktop platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants