Executing SUID binaries not possible in v5

[Eugeny]

Since v5, it's no longer possible to start setuid binaries from within an Electron renderer process - even with sandboxing disabled.

[vladimiry]

See:

• [email protected] fails with error: No usable sandbox! #16631
• The SUID sandbox helper binary was found, but is not configured correctly #17972

[Eugeny]

Just to clarify, I'm not taking about the SUID sandbox - just about using child_process to launch sudo.

[nornagon]

What error are you seeing (if any)?

What OS are you using (distro + version, kernel version)?

Can you please make a small repro (e.g. using fiddle)?

Thanks!

[Eugeny]

The error is

sudo: effective uid is not 0, is /usr/bin/sudo on a file system with the 'nosuid' option set
      or an NFS file system without root privileges?

And executing a custom binary instead shows that the SUID bit is not respected for child processes launched from the Electron renderer process.

The OS is a fresh latest stable Elementary - I'll get you a repro tonight.

[nornagon]

Just to ask the obvious question, is /usr/bin/sudo on a file system with the 'nosuid' option by any chance?

[Eugeny]

Fiddle: https://gist.github.com/Eugeny/ec7be271806e2c16838977337b987606#file-renderer-js-L1

Result:

/usr/bin/sudo runs normally from the OS terminal.

[Eugeny]

@nornagon have you been able to test the repro?

[Muffinman]

I'm seeing this issue on Ubuntu 18.04 when using sudo-prompt in a simple example app.

[barretron]

I'm running electron with --no-sandbox --no-setuid-sandbox and still seeing this, but the following works:

const req = require('electron').remote.require;
const { execSync } = req('child_process');
execSync('sudo ls', { input: 'your-password\n' });

There were some deprecations in v5 related to require('child_process') when sandbox: true, but seems like it's not working even in an unsandboxed environment.

Note: this is a Linux only issue for me, tested on Debian 9.

[Eugeny]

Can confirm that this is a Linux-only issue, macOS works fine.

[NuSkooler]

Any traction on this? This has essentially rendered the Terminus project useless for many on Linux.

[ghost]

Apparently, the root cause of this problem is this commit which was first introduced in v5.0.0-beta1.
When the allow_new_privs flag is cleared in an Electron process, NO_NEW_PRIVS bit is set in all the descendant processes including node-pty rendering them unable to run SUID binaries. This bit is irrevocable, so as long as node-pty is launched from the same Electron process, calling syscalls and such on the node-pty process as a symptomatic treatment does not work.
I am not familiar with the Electron internals so I am not sure what the commit is for, or how reverting the commit would break things. If the commit cannot be reverted, the only way to fix this issue is to launch node-pty elsewhere.

[ghost]

Looking at #15229 again, it seems that the commit was not necessary and the patch was just considered obsolete. Reverting #15229 sounds harmless, could you comment on this @zcbenz if you have time?

[nornagon]

Ah, thanks for the detective work! Yes, it seems like that patch could cause this issue. I'm not sure that reverting it is the right call though, because we'd like to keep that protection in place for sandboxed processes (which can't call execSync anyway). We could look into an alternative technique that would allow new privileges from unsandboxed renderers, though.

As a workaround, would it be possible to try launching the process from the main process instead of the renderer? The main process shouldn't have that flag set.

[Eugeny]

Unfortunately not, as IPC would kill performance.

[Eugeny]

@nornagon thanks for making the patch!