Skip to content

Drop real UID/GID too so GTK's bwrap image loaders can run (#1310) - #1338

Open
devin-ai-integration[bot] wants to merge 1 commit into
masterfrom
devin/1793039564-drop-privs-ruid
Open

devin-ai-integration[bot] wants to merge 1 commit into
masterfrom
devin/1793039564-drop-privs-ruid

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Summary

Second half of #1310, on top of the SIGCHLD fix in #1315 (the two are independent and can merge in either order).

drop_privs() only changed the effective IDs, so after "Accept" in the GTK setup screen ettercap runs as ruid=0 / euid=65534. Every child inherits that mixed state. bubblewrap treats ruid != euid as a setuid invocation and refuses to start (setuid use of bubblewrap is not supported, or Unexpected setuid user 65534, should be 0 on older bwrap). As a result glycin's sandboxed image loader exits with status 1 and GTK aborts in ensure_surface_for_gicon(). This is the Loader process exited early with status '1' crash that @koeppea reproduced on Kali.

Fix: when setresuid() is available, also set the real IDs and keep only the saved set-user-ID as root:

/* drop_privs() */                    /* regain_privs() */
setresgid(gid, gid, 0);               setresuid(0, 0, 0);
setresuid(uid, uid, 0);               setresgid(0, 0, 0);
// was: setegid(gid); seteuid(uid);   // was: seteuid(0);

After the drop: Uid: 65534 65534 0 65534. exec() resets the saved ID to the effective one, so children (bwrap, glycin) run fully as nobody. regain_privs() still works because the saved UID is 0. It also now restores ruid=0, so the getuid() checks in restore_ip_forward() / restore_ipv6_forward() still pass.

Other changes:

  • HAVE_SETRESUID is detected via check_function_exists. Platforms without it (macOS) keep the old seteuid()/setegid() path. bwrap only exists on Linux anyway.
  • include/ec.h: <unistd.h> is now included while _GNU_SOURCE/__USE_GNU are still defined. Without that, glibc doesn't declare setresuid()/setresgid().
  • Side effects: other processes running as the drop user (nobody) can now signal ettercap, because its real UID is nobody. The remote_browser plugin's getuid() == 0 branch no longer runs after the drop. Its setuid(1000) already failed with EPERM there (euid was 65534), so the browser ran as nobody in practice anyway.

How to reproduce / test (Debian testing, no Kali needed)

Deterministic bwrap check (from @0x03c1):

$ B="bwrap --unshare-all --die-with-parent --chdir / --ro-bind /usr /usr --dev /dev --symlink usr/lib /lib --symlink usr/lib64 /lib64 --symlink usr/bin /bin /usr/bin/true"
$ sudo setpriv --ruid 0     --euid 65534 --regid 65534 --clear-groups $B   # exit 1  (old drop_privs state)
$ sudo setpriv --ruid 65534 --euid 65534 --regid 65534 --clear-groups $B   # exit 0  (new drop_privs state)

End-to-end GUI: I used a debian:testing container (--privileged so bwrap can create user namespaces) with glycin-loaders 2.x, bubblewrap, libgtk-3-dev, Xvfb and xdotool. GTK3 build, ettercap -G as root. Steps: click Accept (this is where privileges are dropped), leave the GUI idle for 90 s so glycin tears down its loaders, then open the header-bar menus so new icons have to be loaded.

build UIDs after Accept result
master 0 65534 0 65534 Aborted: Gtk:ERROR ... gtkiconhelper.c:495 ... image-missing.svg: Loader process exited early
this PR 65534 65534 0 65534 still running, menus/host list work
this PR + #1315 65534 65534 0 65534 still running

Also checked ettercap -T with ip_forward=1: it reads 0 while ettercap runs and is back to 1 after SIGTERM. That confirms the regain_privs() atexit path still works with the new IDs.

Link to Devin session: https://app.devin.ai/sessions/ea7a02d2d699413da9dfa2d8482944bf
Open in Devin Desktop: https://app.devin.ai/desktop/session/ea7a02d2d699413da9dfa2d8482944bf?variant=devin
Requested by: @eaescob

drop_privs() only changed the effective IDs, leaving the process at
ruid=0/euid=65534. Children inherit that mixed state and bubblewrap
treats it as a setuid invocation and refuses to start, so glycin's
sandboxed image loaders exit with status 1 and GTK aborts in
ensure_surface_for_gicon().

Use setresuid()/setresgid() where available to set the real and
effective IDs to the unprivileged user while keeping the saved
set-user-ID at 0, so regain_privs() (redirect commands, atexit
restore of ip_forward) keeps working. Platforms without setresuid()
keep the old seteuid()/setegid() behaviour.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Devin Review

Comment thread src/ec_utils.c

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant