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

--build clears the environment #4460

Closed
7 tasks
rusty-snake opened this issue Aug 9, 2021 · 3 comments · Fixed by #4467
Closed
7 tasks

--build clears the environment #4460

rusty-snake opened this issue Aug 9, 2021 · 3 comments · Fixed by #4467
Labels
bug Something isn't working

Comments

@rusty-snake
Copy link
Collaborator

Bug and expected behavior

  • Describe the bug.

--build clears the env. Only a few firejail set variables and LANG, PATH, DISPLAY and SHELL are kept.

$ firejail --build printenv
LANG=de_DE.UTF-8
PATH=/home/rusty-snake/.config/firecfg.py/overrides/bin:/etc/firecfg.py/overrides/bin:/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin:/home/rusty-snake/.local/bin:/home/rusty-snake/.cargo/bin
DISPLAY=:0
QT_X11_NO_MITSHM=1
QML_DISABLE_DISK_CACHE=1
container=firejail
SHELL=/bin/bash
KDE_FORK_SLAVES=1
PROMPT_COMMAND=:
FIREJAIL_QUIET=yes

This makes --build unusable for programs which assume variables like HOME are set (os.environ["HOME"]).

  • What did you expect to happen?

Variables like HOME are still present.

Reproduce
Steps to reproduce the behavior:

  1. Run in bash firejail --build printenv
  2. See error nearly empty environment

Environment

  • Linux distribution and version: Fedora 34
  • Firejail version: 0.9.66+3836131f-dirty

Checklist

  • The profile (and redirect profile if exists) hasn't already been fixed upstream.
  • The program has a profile. (If not, request one in https://github.com/netblue30/firejail/issues/1139)
  • I have performed a short search for similar issues (to avoid opening a duplicate).
  • If it is a AppImage, --profile=PROFILENAME is used to set the right profile.
  • Used LC_ALL=en_US.UTF-8 LANG=en_US.UTF-8 PROGRAM to get english error-messages.
  • I'm aware of browser-allow-drm yes/browser-disable-u2f no in firejail.config to allow DRM/U2F in browsers.
  • This is not a question. Questions should be asked in https://github.com/netblue30/firejail/discussions.
@jmetrius
Copy link
Contributor

firejail applies a filtered set of environment variables before handing off to fbuilder which takes care of the actual profile building. fbuilder itself does nothing to restore the original environment before starting the sandbox via firejail

static void run_builder(int argc, char **argv) {
EUID_ASSERT();
(void) argc;
// drop privileges
if (setresgid(-1, getgid(), getgid()) != 0)
errExit("setresgid");
if (setresuid(-1, getuid(), getuid()) != 0)
errExit("setresuid");
assert(env_get("LD_PRELOAD") == NULL);
assert(getenv("LD_PRELOAD") == NULL);
umask(orig_umask);
// restore some environment variables
env_apply_whitelist_sbox();
argv[0] = LIBDIR "/firejail/fbuilder";
execvp(argv[0], argv);
perror("execvp");
exit(1);
}

Using env_apply_all() at line 940 gives the expected behaviour for firejail --build printenv.

However I don't understand what the intention for handing over a sanitized environment for fbuilder actually is. If restoring the original environment from fbuilder should be more desirable, the solution doesn't seem that trivial.

@rusty-snake
Copy link
Collaborator Author

Maybe it is just a code-path forgotten in #3322. @topimiettinen?

@topimiettinen
Copy link
Collaborator

I haven't used --build, but if fbuilder needs all environment variables, the fix should be simply to change env_apply_whitelist_sbox() to env_apply_all().

topimiettinen added a commit to topimiettinen/firejail that referenced this issue Aug 12, 2021
@topimiettinen topimiettinen added the bug Something isn't working label Aug 12, 2021
topimiettinen added a commit that referenced this issue Aug 16, 2021
kmk3 added a commit to kmk3/firejail that referenced this issue Jan 26, 2022
Note: They are added in the order that the issues were fixed/closed.

Note2: The issues were found through the following url:

https://github.com/netblue30/firejail/issues?q=is%3Aclosed+label%3Abug+-label%3Asecurity+closed%3A%3E2021-06-29+

The date used is the release date of 0.9.66, so in theory the query
should return every bug closed after that.  Security-related issues are
excluded because they will be added separately.

Note3: All issues other than netblue30#4328 were fixed before 0.9.68rc1.

Relates to netblue30#2758 netblue30#4235 netblue30#4328 netblue30#4387 netblue30#4395 netblue30#4460 netblue30#4467 netblue30#4558 netblue30#4560 netblue30#4586.
@kmk3 kmk3 added this to To do in Release 0.9.68 via automation Jan 27, 2022
@kmk3 kmk3 moved this from To do to To Document (RELNOTES/man) in Release 0.9.68 Jan 27, 2022
@kmk3 kmk3 moved this from To Document (RELNOTES/man) to Done (on RELNOTES) in Release 0.9.68 Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Release 0.9.68
  
Done (on RELNOTES)
Development

Successfully merging a pull request may close this issue.

3 participants