Fix argv injection in _create_user and gpasswd loop (#4473)
* Fix argv injection in _create_user and gpasswd loop Use argv list with run() instead of f-string interpolation into SysCommand, add debug logging on failure. * Extract _chroot_argv helper and harden user/file ops Address svartkanin's review on #4473: factor the ['arch-chroot', '-S', str(self.target), ...] boilerplate into a private Installer._chroot_argv() helper, and migrate the seven existing argv-form call sites to it (useradd, gpasswd, chpasswd, chsh, chown, snapper-create-config, grub-install). Two related hardening tweaks while in the area: - Raise gpasswd failure log from debug() to warn(). The group-add loop has no return-False feedback channel for the caller, so a silent debug() means a half-configured user looks like a successful install. - Add `--` end-of-options separator for useradd and chown so a username or path starting with `-` cannot smuggle flags. The TUI validates usernames, but parse_arguments() in models/users.py does not, so config.json is the residual hole; this closes it for these two sites at zero cost.
This commit is contained in:
parent
d836ab0a66
commit
9e05260df5
|
|
@ -739,6 +739,9 @@ class Installer:
|
|||
|
||||
return self.run_command(cmd, peek_output=peek_output)
|
||||
|
||||
def _chroot_argv(self, *args: str) -> list[str]:
|
||||
return ['arch-chroot', '-S', str(self.target), *args]
|
||||
|
||||
def drop_to_shell(self) -> None:
|
||||
subprocess.check_call(f'arch-chroot {self.target}', shell=True)
|
||||
|
||||
|
|
@ -987,17 +990,7 @@ class Installer:
|
|||
}
|
||||
|
||||
for config_name, mountpoint in snapper.items():
|
||||
command = [
|
||||
'arch-chroot',
|
||||
'-S',
|
||||
str(self.target),
|
||||
'snapper',
|
||||
'--no-dbus',
|
||||
'-c',
|
||||
config_name,
|
||||
'create-config',
|
||||
mountpoint,
|
||||
]
|
||||
command = self._chroot_argv('snapper', '--no-dbus', '-c', config_name, 'create-config', mountpoint)
|
||||
|
||||
try:
|
||||
SysCommand(command, peek_output=True)
|
||||
|
|
@ -1336,13 +1329,7 @@ class Installer:
|
|||
|
||||
boot_dir = Path('/boot')
|
||||
|
||||
command = [
|
||||
'arch-chroot',
|
||||
'-S',
|
||||
str(self.target),
|
||||
'grub-install',
|
||||
'--debug',
|
||||
]
|
||||
command = self._chroot_argv('grub-install', '--debug')
|
||||
|
||||
if SysInfo.has_uefi():
|
||||
if not efi_partition:
|
||||
|
|
@ -1922,16 +1909,17 @@ class Installer:
|
|||
if not handled_by_plugin:
|
||||
info(f'Creating user {user.username}')
|
||||
|
||||
cmd = 'useradd -m'
|
||||
cmd = self._chroot_argv('useradd', '-m')
|
||||
|
||||
if user.sudo:
|
||||
cmd += ' -G wheel'
|
||||
cmd += ['-G', 'wheel']
|
||||
|
||||
cmd += f' {user.username}'
|
||||
cmd += ['--', user.username]
|
||||
|
||||
try:
|
||||
self.arch_chroot(cmd)
|
||||
except SysCallError as err:
|
||||
run(cmd)
|
||||
except CalledProcessError as err:
|
||||
debug(f'Error creating user {user.username}: {err}')
|
||||
raise SystemError(f'Could not create user inside installation: {err}')
|
||||
|
||||
for plugin in plugins.values():
|
||||
|
|
@ -1942,7 +1930,11 @@ class Installer:
|
|||
self.set_user_password(user)
|
||||
|
||||
for group in user.groups:
|
||||
self.arch_chroot(f'gpasswd -a {user.username} {group}')
|
||||
cmd = self._chroot_argv('gpasswd', '-a', user.username, group)
|
||||
try:
|
||||
run(cmd)
|
||||
except CalledProcessError as err:
|
||||
warn(f'Failed to add {user.username} to group {group}: {err}')
|
||||
|
||||
if user.sudo:
|
||||
self.enable_sudo(user)
|
||||
|
|
@ -1957,7 +1949,7 @@ class Installer:
|
|||
return False
|
||||
|
||||
input_data = f'{user.username}:{enc_password}'.encode()
|
||||
cmd = ['arch-chroot', '-S', str(self.target), 'chpasswd', '--encrypted']
|
||||
cmd = self._chroot_argv('chpasswd', '--encrypted')
|
||||
|
||||
try:
|
||||
run(cmd, input_data=input_data)
|
||||
|
|
@ -1969,7 +1961,7 @@ class Installer:
|
|||
def user_set_shell(self, user: str, shell: str) -> bool:
|
||||
info(f'Setting shell for {user} to {shell}')
|
||||
|
||||
cmd = ['arch-chroot', '-S', str(self.target), 'chsh', '-s', shell, user]
|
||||
cmd = self._chroot_argv('chsh', '-s', shell, user)
|
||||
try:
|
||||
run(cmd)
|
||||
return True
|
||||
|
|
@ -1979,7 +1971,7 @@ class Installer:
|
|||
|
||||
def chown(self, owner: str, path: str, options: list[str] | None = None) -> bool:
|
||||
options = options or []
|
||||
cmd = ['arch-chroot', '-S', str(self.target), 'chown', *options, owner, path]
|
||||
cmd = self._chroot_argv('chown', *options, '--', owner, path)
|
||||
try:
|
||||
run(cmd)
|
||||
return True
|
||||
|
|
|
|||
Loading…
Reference in New Issue