Uploaded image for project: 'RHEL Conversions'
  1. RHEL Conversions
  2. RHELC-408

Security hardening: Change run_subprocess to take a list rather than a string - el8

XMLWordPrintable

    • Icon: Task Task
    • Resolution: Won't Do
    • Icon: Normal Normal
    • 0.26
    • 8.6
    • convert2rhel
    • None
    • False
    • False
    • 8

      Background

      We have a helper function, utils.run_subproccess(), for running other programs.   It takes the command to execute as a string, splits it, and then passes the list to python's subprocess.Popen() to run.

       

      This is okay as long as the code calling run_subprocess() takes care to use shlex.quote() to quote the arguments it is passing. Otherwise things like spaces in filenames will be incorrectly split.  Passing the command as a list instead of a string mitigates this by having the programmer calling run_subprocess()put each argument into a separate list element rather than relying on run_subprocess() to split it.

       

      In depth explanation if more information is desired:  https://gist.github.com/abadger/ea234ee7a7238b343bcfd6537d6e92e7

      Work to be performed

      • Change the run_subprocess() code to take a list instead of a string (1 hour, remove the splitting code, modify the docstring, and change the logging to join the list which was passed in.)
      • Change all callers of run_subprocess() to send a list instead of a string.
      • Modify the use of run_subprocess() in the unit_tests()
        • There are currently 134 mentions of run_subprocess() in the unit tests.
        • The majority of these will not need changes.  We're mocking the function to return a specific value (the mock won't care how the function is called)..
        • Although not a majority, there are many tests where we check that run_subprocess() is called with the arguments we expect.  It will take time to work through all of these but the change is simply to look for the expected arguments as a list instead of a string.

              aanglin@redhat.com Andrew Anglin
              tkuratom@redhat.com Toshio Kuratomi
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Created:
                Updated:
                Resolved: