pmbootstrap/pmb/chroot/user.py

40 lines
1.3 KiB
Python
Raw Normal View History

2023-01-22 18:11:10 +00:00
# Copyright 2023 Oliver Smith
# SPDX-License-Identifier: GPL-3.0-or-later
2017-05-26 20:08:45 +00:00
import pmb.chroot.root
Properly escape commands in pmb.chroot.user() (#1316) ## Introduction In #1302 we noticed that `pmb.chroot.user()` does not escape commands properly: When passing one string with spaces, it would pass them as two strings to the chroot. The use case is passing a description with a space inside to `newapkbuild` with `pmboostrap newapkbuild`. This is not a security issue, as we don't pass strings from untrusted input to this function. ## Functions for running commands in pmbootstrap To put the rest of the description in context: We have four high level functions that run commands: * `pmb.helpers.run.user()` * `pmb.helpers.run.root()` * `pmb.chroot.root()` * `pmb.chroot.user()` In addition, one low level function that the others invoke: * `pmb.helpers.run.core()` ## Flawed test case The issue described above did not get detected for so long, because we have a test case in place since day one, which verifies that all of the functions above escape everything properly: * `test/test_shell_escape.py` So the test case ran a given command through all these functions, and compared the result each time. However, `pmb.chroot.root()` modified the command variable (passed by reference) and did the escaping already, which means `pmb.chroot.user()` running directly afterwards only returns the right output when *not* doing any escaping. Without questioning the accuracy of the test case, I've escaped commands and environment variables with `shlex.quote()` *before* passing them to `pmb.chroot.user()`. In retrospective this does not make sense at all and is reverted with this commit. ## Environment variables By coincidence, we have only passed custom environment variables to `pmb.chroot.user()`, never to the other high level functions. This only worked, because we did not do any escaping and the passed line gets executed as shell command: ``` $ MYENV=test echo test2 test 2 ``` If it was properly escaped as one shell command: ``` $ 'MYENV=test echo test2' sh: MYENV=test echo test2: not found ``` So doing that clearly doesn't work anymore. I have added a new `env` parameter to `pmb.chroot.user()` (and to all other high level functions for consistency), where environment variables can be passed as a dictionary. Then the function knows what to do and we end up with properly escaped commands and environment variables. ## Details * Add new `env` parameter to all high level command execution functions * New `pmb.helpers.run.flat_cmd()` function, that takes a command as list and environment variables as dict, and creates a properly escaped flat string from the input. * Use that function for proper escaping in all high level exec funcs * Don't escape commands *before* passing them to `pmb.chroot.user()` * Describe parameters of the command execution functions * `pmbootstrap -v` writes the exact command to the log that was executed (in addition to the simplified form we always write down for readability) * `test_shell_escape.py`: verify that the command passed by reference has not been modified, add a new test for strings with spaces, add tests for new function `pmb.helpers.run.flat_cmd()` * Remove obsolete commend in `pmb.chroot.distccd` about environment variables, because we don't use any there anymore * Add `TERM=xterm` to default environment variables in the chroot, so running ncurses applications like `menuconfig` and `nano` works out of the box
2018-03-10 22:58:39 +00:00
import pmb.helpers.run
2017-05-26 20:08:45 +00:00
def user(args, cmd, suffix="native", working_dir="/", output="log",
output_return=False, check=None, env={}, auto_init=True):
2017-05-26 20:08:45 +00:00
"""
Properly escape commands in pmb.chroot.user() (#1316) ## Introduction In #1302 we noticed that `pmb.chroot.user()` does not escape commands properly: When passing one string with spaces, it would pass them as two strings to the chroot. The use case is passing a description with a space inside to `newapkbuild` with `pmboostrap newapkbuild`. This is not a security issue, as we don't pass strings from untrusted input to this function. ## Functions for running commands in pmbootstrap To put the rest of the description in context: We have four high level functions that run commands: * `pmb.helpers.run.user()` * `pmb.helpers.run.root()` * `pmb.chroot.root()` * `pmb.chroot.user()` In addition, one low level function that the others invoke: * `pmb.helpers.run.core()` ## Flawed test case The issue described above did not get detected for so long, because we have a test case in place since day one, which verifies that all of the functions above escape everything properly: * `test/test_shell_escape.py` So the test case ran a given command through all these functions, and compared the result each time. However, `pmb.chroot.root()` modified the command variable (passed by reference) and did the escaping already, which means `pmb.chroot.user()` running directly afterwards only returns the right output when *not* doing any escaping. Without questioning the accuracy of the test case, I've escaped commands and environment variables with `shlex.quote()` *before* passing them to `pmb.chroot.user()`. In retrospective this does not make sense at all and is reverted with this commit. ## Environment variables By coincidence, we have only passed custom environment variables to `pmb.chroot.user()`, never to the other high level functions. This only worked, because we did not do any escaping and the passed line gets executed as shell command: ``` $ MYENV=test echo test2 test 2 ``` If it was properly escaped as one shell command: ``` $ 'MYENV=test echo test2' sh: MYENV=test echo test2: not found ``` So doing that clearly doesn't work anymore. I have added a new `env` parameter to `pmb.chroot.user()` (and to all other high level functions for consistency), where environment variables can be passed as a dictionary. Then the function knows what to do and we end up with properly escaped commands and environment variables. ## Details * Add new `env` parameter to all high level command execution functions * New `pmb.helpers.run.flat_cmd()` function, that takes a command as list and environment variables as dict, and creates a properly escaped flat string from the input. * Use that function for proper escaping in all high level exec funcs * Don't escape commands *before* passing them to `pmb.chroot.user()` * Describe parameters of the command execution functions * `pmbootstrap -v` writes the exact command to the log that was executed (in addition to the simplified form we always write down for readability) * `test_shell_escape.py`: verify that the command passed by reference has not been modified, add a new test for strings with spaces, add tests for new function `pmb.helpers.run.flat_cmd()` * Remove obsolete commend in `pmb.chroot.distccd` about environment variables, because we don't use any there anymore * Add `TERM=xterm` to default environment variables in the chroot, so running ncurses applications like `menuconfig` and `nano` works out of the box
2018-03-10 22:58:39 +00:00
Run a command inside a chroot as "user". We always use the BusyBox
implementation of 'su', because other implementations may override the PATH
environment variable (#1071).
:param env: dict of environment variables to be passed to the command, e.g.
{"JOBS": "5"}
:param auto_init: automatically initialize the chroot
See pmb.helpers.run_core.core() for a detailed description of all other
arguments and the return value.
2017-05-26 20:08:45 +00:00
"""
if "HOME" not in env:
env["HOME"] = "/home/pmos"
Properly escape commands in pmb.chroot.user() (#1316) ## Introduction In #1302 we noticed that `pmb.chroot.user()` does not escape commands properly: When passing one string with spaces, it would pass them as two strings to the chroot. The use case is passing a description with a space inside to `newapkbuild` with `pmboostrap newapkbuild`. This is not a security issue, as we don't pass strings from untrusted input to this function. ## Functions for running commands in pmbootstrap To put the rest of the description in context: We have four high level functions that run commands: * `pmb.helpers.run.user()` * `pmb.helpers.run.root()` * `pmb.chroot.root()` * `pmb.chroot.user()` In addition, one low level function that the others invoke: * `pmb.helpers.run.core()` ## Flawed test case The issue described above did not get detected for so long, because we have a test case in place since day one, which verifies that all of the functions above escape everything properly: * `test/test_shell_escape.py` So the test case ran a given command through all these functions, and compared the result each time. However, `pmb.chroot.root()` modified the command variable (passed by reference) and did the escaping already, which means `pmb.chroot.user()` running directly afterwards only returns the right output when *not* doing any escaping. Without questioning the accuracy of the test case, I've escaped commands and environment variables with `shlex.quote()` *before* passing them to `pmb.chroot.user()`. In retrospective this does not make sense at all and is reverted with this commit. ## Environment variables By coincidence, we have only passed custom environment variables to `pmb.chroot.user()`, never to the other high level functions. This only worked, because we did not do any escaping and the passed line gets executed as shell command: ``` $ MYENV=test echo test2 test 2 ``` If it was properly escaped as one shell command: ``` $ 'MYENV=test echo test2' sh: MYENV=test echo test2: not found ``` So doing that clearly doesn't work anymore. I have added a new `env` parameter to `pmb.chroot.user()` (and to all other high level functions for consistency), where environment variables can be passed as a dictionary. Then the function knows what to do and we end up with properly escaped commands and environment variables. ## Details * Add new `env` parameter to all high level command execution functions * New `pmb.helpers.run.flat_cmd()` function, that takes a command as list and environment variables as dict, and creates a properly escaped flat string from the input. * Use that function for proper escaping in all high level exec funcs * Don't escape commands *before* passing them to `pmb.chroot.user()` * Describe parameters of the command execution functions * `pmbootstrap -v` writes the exact command to the log that was executed (in addition to the simplified form we always write down for readability) * `test_shell_escape.py`: verify that the command passed by reference has not been modified, add a new test for strings with spaces, add tests for new function `pmb.helpers.run.flat_cmd()` * Remove obsolete commend in `pmb.chroot.distccd` about environment variables, because we don't use any there anymore * Add `TERM=xterm` to default environment variables in the chroot, so running ncurses applications like `menuconfig` and `nano` works out of the box
2018-03-10 22:58:39 +00:00
flat_cmd = pmb.helpers.run.flat_cmd(cmd, env=env)
cmd = ["busybox", "su", "pmos", "-c", flat_cmd]
return pmb.chroot.root(args, cmd, suffix, working_dir, output,
output_return, check, {}, auto_init)
def exists(args, username, suffix="native"):
"""
Checks if username exists in the system
:param username: User name
:returns: bool
"""
output = pmb.chroot.root(args, ["getent", "passwd", username],
suffix, output_return=True, check=False)
return len(output) > 0