Properly escape commands in pmb.chroot.user()

Created by: ollieparanoid

Introduction

In #1302 (closed) 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