Closed dchackett closed 6 years ago
I think this is fixed in the new modularize branch. No arbitrary code is executed; instead, the old "run_script" tasks are now all specific "XRunner" tasks, that call an instance of a TaskRunner which has an "execute" procedure. The binary to be used by a particular runner is specified within the Python scripts (in local_taxi.py), so it can't be replaced arbitrarily.
Have a look and see if you spot any other holes; I won't close the issue yet.
The current model has tasks run by Task objects rather than calls to external runner scripts. I think this scheme plugs this security hole about as well as we can at our current level of paranoia. Closing issue.
Currently, taxis are allowed to call arbitrary python scripts with arbitrary command line arguments. This is obviously a security issue.
For a fun example of a potential future exploit, consider a case where taxis talk to some external database rather than a local sqlite database. The attacker cannot access the system where the runner scripts are stored, but can access the database. By changing a run_script task such that the script it calls is "", and the command line arguments dict looks like {'c' : "ARBITRARY_PYTHON_CODE"}, the attacker can have taxi make the call: "python -c "ARBITRARY PYTHON CODE"".
The above case can obviously be protected against, but the structure is fundamentally flawed from a security standpoint. How can we call runner scripts more securely but equally flexibly?