mirror of
https://github.com/netblue30/firejail.git
synced 2026-05-15 14:16:14 -06:00
[PR #4326] [MERGED] cmdline.c: optionally quote the resulting command line #5112
Labels
No labels
LTS merge
LTS merge
bug
bug
converted-to-discussion
doc-todo
documentation
duplicate
enhancement
file-transfer
firecfg
firejail-in-firejail
firetools
graphics
help wanted
information_old
installation
invalid
modif
moved
needinfo
networking
notabug
notourbug
old-version
overlayfs
packaging
profile-request
pull-request
question
question_old
removal
runtime-permissions
sandbox-ipc
security
stale
wiki
wiki
wontfix
wordpress
workaround
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: github-starred/firejail#5112
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
📋 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:
master← Head:pr/master/dont-quote-all-cmdlines📝 Commits (1)
afb7d07cmdline.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
sshdinvokes 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:
and ssh'ing in a non-interactive multi-token command with firejail as the login shell (assume a trivial
username: --shell=/bin/bashline in/etc/firejail/login.users):The difference is that in the
sshcase, the localsshdonsomenodewill invoke the following:argv[0]:/usr/bin/firejailargv[1]:-cargv[2]:echo hello worldNotice how
echo hello worldis a single token (argv[2]).For simplicity of explanation, let's take
ssh/sshdout of the situation and just explain two different command line cases.Case 1: a single token
Consider:
This is equivalent to what
sshddoes:echo hello worldis a single token (argv[3], in this example).Firejail ultimately invokes
src/firejail/cmdline.c:quote_cmdline()at4522ccb4ef/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/bashargv[1]:-cargv[2]:'echo hello world'With the extra quotes, Bash won't split up
argv[2], and therefore won't find an intrinsic or executable namedecho hello world. Bash then rightfully generates an error.Case 2: multiple tokens
Consider:
Note that
echohelloworldare all their ownargvtokens (argv[3]throughargv[5]in this example). Firejail'squote_cmdline()behavior is therefore different; bash is ultimately executed as:argv[0]:/bin/bashargv[1]:-cargv[2]:'echo' 'hello' 'world'Bash therefore is able to split up the resulting
argv[2], find anechointrinsic or executable, and things proceed as expected (i.e.,hello worldis 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 bysshdor not.This PR uses the already-set
parent_sshdflag to tellbuild_cmdline()(which, in turn, invokesquote_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/bashargv[1]:-cargv[2]:echo hello worldWhich, 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.