[PR #4326] [MERGED] cmdline.c: optionally quote the resulting command line #5112

Closed
opened 2026-05-05 10:32:33 -06:00 by gitea-mirror · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netblue30/firejail/pull/4326
Author: @jsquyres
Created: 6/2/2021
Status: Merged
Merged: 6/4/2021
Merged by: @netblue30

Base: masterHead: pr/master/dont-quote-all-cmdlines


📝 Commits (1)

  • afb7d07 cmdline.c: optionally quote the resulting command line

📊 Changes

5 files changed (+24 additions, -21 deletions)

View changed files

📝 src/firejail/cmdline.c (+17 -15)
📝 src/firejail/firejail.h (+2 -2)
📝 src/firejail/join.c (+1 -1)
📝 src/firejail/main.c (+3 -2)
📝 src/firejail/no_sandbox.c (+1 -1)

📄 Description

The commit message contains an abbreviated version of this explanation. This PR is a possible solution for #1644, #887, and #1817.


Short version

sshd invokes firejail slightly differently than how firejail is normally invoked on the command line. One possible fix is to recognize that firejail was launched by sshd and slightly modify its quoting behavior.

More details

There's a difference between invoking firejail with a multi-token command:

# Using --quiet for brevity
$ firejail --quiet echo hello world
hello world

and ssh'ing in a non-interactive multi-token command with firejail as the login shell (assume a trivial username: --shell=/bin/bash line in /etc/firejail/login.users):

$ ssh username@othernode echo hello world
/bin/bash: echo hello world: command not found

The difference is that in the ssh case, the local sshd on somenode will invoke the following:

  • argv[0]: /usr/bin/firejail
  • argv[1]: -c
  • argv[2]: echo hello world

Notice how echo hello world is a single token (argv[2]).

For simplicity of explanation, let's take ssh/sshd out of the situation and just explain two different command line cases.

Case 1: a single token

Consider:

# Using --quiet for brevity
$ firejail --quiet -c "echo hello world"
/bin/bash: echo hello world: command not found

This is equivalent to what sshd does: echo hello world is a single token (argv[3], in this example).

Firejail ultimately invokes src/firejail/cmdline.c:quote_cmdline() at 4522ccb4ef/src/firejail/cmdline.c (L67)

quote_cmdline() adds an additional set of single quotes around tokens. The user's shell -- let's assume it's Bash -- is ultimately executed as:

  • argv[0]: /bin/bash
  • argv[1]: -c
  • argv[2]: 'echo hello world'

With the extra quotes, Bash won't split up argv[2], and therefore won't find an intrinsic or executable named echo hello world . Bash then rightfully generates an error.

Case 2: multiple tokens

Consider:

# Using --quiet for brevity
$ firejail --quiet -c echo hello world
hello world

Note that echo hello world are all their own argv tokens (argv[3] through argv[5] in this example). Firejail's quote_cmdline() behavior is therefore different; bash is ultimately executed as:

  • argv[0]: /bin/bash
  • argv[1]: -c
  • argv[2]: 'echo' 'hello' 'world'

Bash therefore is able to split up the resulting argv[2], find an echo intrinsic or executable, and things proceed as expected (i.e., hello world is emitted and we get an exit status of 0).

Proposed solution

One possible solution is to have quote_cmdline() behave differently depending on whether Firejail was invoked by sshd or not.

This PR uses the already-set parent_sshd flag to tell build_cmdline() (which, in turn, invokes quote_cmdline()) to not add extra quotes around the all-the-tokens-are-in-a-single-argv[x] token. Bash is therefore ultimately invoked as:

  • argv[0]: /bin/bash
  • argv[1]: -c
  • argv[2]: echo hello world

Which, while this is slightly different than case 2, above (i.e., there's no extra quotes around each sub-token), seems to be sufficient.


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/netblue30/firejail/pull/4326 **Author:** [@jsquyres](https://github.com/jsquyres) **Created:** 6/2/2021 **Status:** ✅ Merged **Merged:** 6/4/2021 **Merged by:** [@netblue30](https://github.com/netblue30) **Base:** `master` ← **Head:** `pr/master/dont-quote-all-cmdlines` --- ### 📝 Commits (1) - [`afb7d07`](https://github.com/netblue30/firejail/commit/afb7d07f667471f4ba5505736915b4307413ef2d) cmdline.c: optionally quote the resulting command line ### 📊 Changes **5 files changed** (+24 additions, -21 deletions) <details> <summary>View changed files</summary> 📝 `src/firejail/cmdline.c` (+17 -15) 📝 `src/firejail/firejail.h` (+2 -2) 📝 `src/firejail/join.c` (+1 -1) 📝 `src/firejail/main.c` (+3 -2) 📝 `src/firejail/no_sandbox.c` (+1 -1) </details> ### 📄 Description The commit message contains an abbreviated version of this explanation. This PR is a possible solution for #1644, #887, and #1817. ----- # Short version `sshd` invokes firejail slightly differently than how firejail is normally invoked on the command line. One possible fix is to recognize that firejail was launched by sshd and slightly modify its quoting behavior. # More details There's a difference between invoking firejail with a multi-token command: ```sh # Using --quiet for brevity $ firejail --quiet echo hello world hello world ``` and ssh'ing in a non-interactive multi-token command with firejail as the login shell (assume a trivial `username: --shell=/bin/bash` line in `/etc/firejail/login.users`): ``` $ ssh username@othernode echo hello world /bin/bash: echo hello world: command not found ``` The difference is that in the `ssh` case, the local `sshd` on `somenode` will invoke the following: * `argv[0]`: `/usr/bin/firejail` * `argv[1]`: `-c` * `argv[2]`: `echo hello world` Notice how `echo hello world` is a single token (`argv[2]`). For simplicity of explanation, let's take `ssh`/`sshd` out of the situation and just explain two different command line cases. ## Case 1: a single token Consider: ```sh # Using --quiet for brevity $ firejail --quiet -c "echo hello world" /bin/bash: echo hello world: command not found ``` This is equivalent to what `sshd` does: `echo hello world` is a single token (`argv[3]`, in this example). Firejail ultimately invokes `src/firejail/cmdline.c:quote_cmdline()` at https://github.com/netblue30/firejail/blob/4522ccb4ef529fe9cafa60178027be139ce0e592/src/firejail/cmdline.c#L67 `quote_cmdline()` adds an additional set of single quotes around tokens. The user's shell -- let's assume it's Bash -- is ultimately executed as: * `argv[0]`: `/bin/bash` * `argv[1]`: `-c` * `argv[2]`: `'echo hello world' ` With the extra quotes, Bash won't split up `argv[2]`, and therefore won't find an intrinsic or executable named `echo hello world `. Bash then rightfully generates an error. ## Case 2: multiple tokens Consider: ```sh # Using --quiet for brevity $ firejail --quiet -c echo hello world hello world ``` Note that `echo` `hello` `world` are all their own `argv` tokens (`argv[3]` through `argv[5]` in this example). Firejail's `quote_cmdline()` behavior is therefore different; bash is ultimately executed as: * `argv[0]`: `/bin/bash` * `argv[1]`: `-c` * `argv[2]`: `'echo' 'hello' 'world' ` Bash therefore is able to split up the resulting `argv[2]`, find an `echo` intrinsic or executable, and things proceed as expected (i.e., `hello world` is emitted and we get an exit status of 0). ## Proposed solution One possible solution is to have `quote_cmdline()` behave differently depending on whether Firejail was invoked by `sshd` or not. This PR uses the already-set `parent_sshd` flag to tell `build_cmdline()` (which, in turn, invokes `quote_cmdline()`) to *not* add extra quotes around the all-the-tokens-are-in-a-single-argv[x] token. Bash is therefore ultimately invoked as: * `argv[0]`: `/bin/bash` * `argv[1]`: `-c` * `argv[2]`: `echo hello world ` Which, while this is slightly different than case 2, above (i.e., there's no extra quotes around each sub-token), seems to be sufficient. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
gitea-mirror 2026-05-05 10:32:33 -06:00
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#5112
No description provided.