Closed Bug 1323302 Opened 7 years ago Closed 6 years ago

Disallow Firefox from running as sudo

Categories

(Toolkit :: Startup and Profile System, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla61
Tracking Status
firefox60 --- verified
firefox61 --- verified

People

(Reporter: benjamin, Assigned: jld)

References

Details

Attachments

(1 file)

If a user runs Firefox as root but using their own home directory, many things become broken for that user, sometimes permanently. We should refuse to launch Firefox like this.

If we can't detect this situation, I think I might be willing to prevent Firefox from launching as root ever, or maybe only when a special envvar is set. It's a really bad idea.

Also related and perhaps in this bug: disable xremote when running as root.
See Also: → 205053
Priority: -- → P3
By comparison:

Chrome: Refuses to start. Returns error: "Running as root without --no-sandbox is not supported."

Internet Explorer (Windows): Forces use of "protected mode" which limits functionality (such as preventing downloads) and which sites may be visited.

There's a large amount of advice online that suggests running Firefox as root is a good thing, or a way to work around other issues that users have.  This is bad for the security of these systems, as any security issue in Firefox will lead to total system compromise.


On Linux/Xorg systems, another user on the same X session can abuse various parts of Firefox in order to gain root: https://bugzilla.mozilla.org/show_bug.cgi?id=576863

There's also user confusion because "running as root is unsupported", but this doesn't seem to be actually enforced anywhere: https://bugzilla.mozilla.org/show_bug.cgi?id=1206441

Ubuntu also has a security issue due to this bug as they invoke Firefox as root when you try to read upgrade release notes: https://bugs.launchpad.net/ubuntu/+source/ubuntu-release-upgrader/+bug/1174007

That can also cause profile permission stomping, leading to that user being unable to use their profile "as them" again.

This seems like an easy win to vastly improve the security of Firefox, and stop people shooting themselves in the foot.
This is somewhat more aggressively broken as of Firefox 60 (bug 1401062 et seq.): content processes are started in new user namespaces in order to access sandboxing features and, as a side-effect, lose any superuser capabilities they previously had.  Among other things, this means that they're subject to file permissions like a normal user that happens to have uid 0, so if the $XAUTHORITY file is owned by a non-root user and has mode 0600 (which is normally the case for "sudo firefox") then they will not be allowed to read it and will fail to authenticate to the X server.  

(Note that this isn't a meaningful security restriction on its own: a process with uid 0 and no capabilities can still write to files owned by uid 0, which includes a lot of scripts and executables that are run with full root powers, and escalate privileges that way.  It just breaks code that *isn't* trying to exploit your system.)

The result is that the browser UI starts, but it can't load anything except for some about: pages (including about:support and about:config, but not about:home).


As for detecting this: checking a handful of “important” files ($HOME, $XAUTHORITY, $XDG_RUNTIME_DIR, the profile dir) against geteuid() shouldn't be difficult.
Assignee: nobody → jld
Priority: P3 → P1
Comment on attachment 8964427 [details]
Bug 1323302 - Refuse to run under sudo or otherwise as root in a non-root user's session.

https://reviewboard.mozilla.org/r/233160/#review238614

::: toolkit/xre/nsAppRunner.cpp:3207
(Diff revision 1)
> +  static const mozilla::Array<const char*, 3> kVars = {
> +    "HOME",
> +    "XDG_RUNTIME_DIR",
> +    "XAUTHORITY",
> +  };

I'm not sure we should check those every time. How about skipping the test if getuid() == geteuid()?

::: toolkit/xre/nsAppRunner.cpp:3213
(Diff revision 1)
> +    "HOME",
> +    "XDG_RUNTIME_DIR",
> +    "XAUTHORITY",
> +  };
> +
> +  if (PR_GetEnv("MOZ_IGNORE_USER_MISMATCH") != nullptr) {

do we really want an escape hatch?
Attachment #8964427 - Flags: review?(mh+mozilla)
(In reply to Mike Hommey [:glandium] from comment #4)
> I'm not sure we should check those every time. How about skipping the test
> if getuid() == geteuid()?

GTK already refuses to start if ruid != euid (or saved uid), so that's not a problem.  I could restrict it to the case where euid == 0 — if someone's doing unusual things with file capabilities and/or securebits then hopefully they know enough to debug what they break, and otherwise a non-root-owned browser probably can't cause too much trouble.

That would still allow running as root when logged in as root, which arguably isn't something we ought to encourage, but it won't directly break things.

> > +  if (PR_GetEnv("MOZ_IGNORE_USER_MISMATCH") != nullptr) {
> 
> do we really want an escape hatch?

On second thought, no, we don't.
Comment on attachment 8964427 [details]
Bug 1323302 - Refuse to run under sudo or otherwise as root in a non-root user's session.

https://reviewboard.mozilla.org/r/233160/#review239488

::: toolkit/xre/nsAppRunner.cpp:3192
(Diff revision 2)
>  }
>  
>  } // anonymous namespace
>  #endif // XP_WIN
>  
> +#ifdef XP_UNIX

You should probably exclude android here.

::: toolkit/xre/nsAppRunner.cpp:3208
(Diff revision 2)
> +    "XDG_RUNTIME_DIR",
> +    "XAUTHORITY",

XDG_RUNTIME_DIR and XAUTHORITY are not going to be relevant on OSX.
Attachment #8964427 - Flags: review?(mh+mozilla) → review+
(In reply to Mike Hommey [:glandium] from comment #7)
> Comment on attachment 8964427 [details]
> > +#ifdef XP_UNIX
> 
> You should probably exclude android here.

Good point.

> ::: toolkit/xre/nsAppRunner.cpp:3208
> (Diff revision 2)
> > +    "XDG_RUNTIME_DIR",
> > +    "XAUTHORITY",
> 
> XDG_RUNTIME_DIR and XAUTHORITY are not going to be relevant on OSX.

Also a good point; thanks.  And $XAUTHORITY probably won't be relevant with a Wayland-only build, now that I think about it.
I'm not sure there are wayland-only builds.
(In reply to Mike Hommey [:glandium] from comment #10)
> I'm not sure there are wayland-only builds.

I think I was confused because of the GL issues in bug 1434574.  So, never mind that part, then.
Pushed by jedavis@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/90a06cdcb48f
Refuse to run under sudo or otherwise as root in a non-root user's session. r=glandium
https://hg.mozilla.org/mozilla-central/rev/90a06cdcb48f
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Comment on attachment 8964427 [details]
Bug 1323302 - Refuse to run under sudo or otherwise as root in a non-root user's session.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1401062 (important sandbox improvements).
[User impact if declined]: Running "sudo firefox", which previously seemed to work but was unsupported, will fail to load content (tab crash on any page) on most Linux distributions; with this patch it will fail to start and print a message 
[Is this code covered by automated tests?]: It's run whenever Firefox starts so we know it's not regressing in normal use, but we're not testing the error case.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: Optional, but "sudo firefox" should do it.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Low risk
[Why is the change risky/not risky?]: This is an unsupported configuration (although that hasn't been communicated very well) and it's already going to break for most of the Linux userbase.  Note that there's no effect on Windows or Mac or Android.
[String changes made/needed]: None.  (There is a message that's printed to the console, but unfortunately it's English-only because localization isn't available in early startup.)
Attachment #8964427 - Flags: approval-mozilla-beta?
Comment on attachment 8964427 [details]
Bug 1323302 - Refuse to run under sudo or otherwise as root in a non-root user's session.

Prevent Firefox from running with sudo to avoid serious functionality impairments. Approved for 60.0b11.
Attachment #8964427 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
I managed to reproduce this issue on Firefox 60.0a1(2018-03-01), under Ubuntu 16.04x64.
The issue is no longer reproducible on Firefox 61.0a1(2018-04-10), or on Firefox 60.0b11.
Tests were performed under Ubuntu 16.04x86 and under Ubuntu 16.04x64.
'Running Firefox Nightly as root in a regular user's session is not supported.' message is displayed in the terminal.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
This change is causing some side-effects in other applications.

For example I have a QT application that uses QDesktopServices::openUrl(url) to open an URL in the systems default browser.
When the QT app is executed with "sudo", the links will not be opened.

In my opinion instead of refusing to start firefox, it should be avoided to write things into the users home directory as root.
(In reply to Alex from comment #18)
> In my opinion instead of refusing to start firefox, it should be avoided to
> write things into the users home directory as root.

Mozilla folks are looking out for their users' security, and you should too.  In my opinion, this is a poor design choice in your application _that you need to fix._  As someone who probably doesn't use your software, I see no reason why my system's security should be compromised to accommodate it, even behind a command-line flag.

See bug 576863 regarding running Firefox as root. Bug 576863 comment #3 specifically details _one_ reason why running any X application as root is a bad idea.

You should be writing your tool to run as a regular user, and use tools like Polkit to invoke specific actions that need to run as root.

I'm quite aware that many Linux applications do not heed this advice (eg: anaconda, gparted, Ubuntu's updater, etc.), but until very recently they have not been forced to.  Picking on ubuntu-release-upgrader[1] as an example, the bug was unactioned for 5 years, until Mozilla folks notified of the upcoming change, and then it was fixed in 4 days.

These sorts of security considerations are very much the way that "the Linux desktop" is going these days:

- https://bugzilla.gnome.org/show_bug.cgi?id=772875#c5 (GNOME are saying this is never supported, and why)
- https://bugzilla.redhat.com/show_bug.cgi?id=1274451 (Wayland are deliberately disallowing running GUI apps as root)

However, I fear that further debate on this is very much out of the scope of Firefox. ;)

[1]: https://bugs.launchpad.net/ubuntu/+source/ubuntu-release-upgrader/+bug/1174007
You need to log in before you can comment on or make changes to this bug.