Skip to content

Fix shell injection in Installer.user_set_shell and Installer.chown#4443

Merged
svartkanin merged 1 commit intoarchlinux:masterfrom
Softer:fix-sh-c-injection-installer
Apr 20, 2026
Merged

Fix shell injection in Installer.user_set_shell and Installer.chown#4443
svartkanin merged 1 commit intoarchlinux:masterfrom
Softer:fix-sh-c-injection-installer

Conversation

@Softer
Copy link
Copy Markdown
Contributor

@Softer Softer commented Apr 18, 2026

Installer.user_set_shell and Installer.chown built shell commands via f-string interpolation inside sh -c "...", allowing command injection via caller-supplied username, shell, path, or options.

Replaced both with argv lists passed directly to run(), matching the pattern already used by user_set_password. The full string is now a single argv element and cannot be shell-parsed.

Both methods are public API of the Installer class and are reachable from custom install.py scripts and plugin code. Any caller that passes unsanitized strings (e.g. a username containing ;, $(), backticks, or spaces) would execute arbitrary shell commands in the chroot. The fix removes the shell layer entirely.

Changes

  • user_set_shell: sh -c "chsh -s {shell} {user}" -> run(['arch-chroot', '-S', target, 'chsh', '-s', shell, user])
  • chown: sh -c 'chown {options} {owner} {cleaned_path}' -> run(['arch-chroot', '-S', target, 'chown', *options, owner, path])
  • Dropped the cleaned_path quote-escape hack (no longer needed).
  • Fixed mutable default argument options: list[str] = [] -> options: list[str] | None = None.
  • Added debug() log on CalledProcessError in both methods.

Tests

  • Unit test with malicious inputs (; rm -rf /, $(id), backticks, spaces, quotes) confirms the entire string stays a single argv element and is never shell-parsed

Use argv list with run() instead of sh -c with f-string interpolation,
fix mutable default argument in chown, add debug logging on failure.
@Softer Softer requested a review from Torxed as a code owner April 18, 2026 09:10
@Softer Softer changed the title Fix shell injection in Installer.user_set_shell and Installer.chown Fix shell injection in Installer.user_set_shell and Installer.chown Apr 18, 2026
@svartkanin svartkanin merged commit 82a46c6 into archlinux:master Apr 20, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants