Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

command injection on the host via the xmlrpc api #141

Closed
jdstrand opened this issue Apr 20, 2012 · 5 comments
Closed

command injection on the host via the xmlrpc api #141

jdstrand opened this issue Apr 20, 2012 · 5 comments
Assignees
Labels
Bug Report Reporting a bug Security

Comments

@jdstrand
Copy link

This is forwarded from https://launchpad.net/bugs/978999:

"It appears as if the power_system method exposed in the xmlrpc api is vulnerable to command injection through either the system handle(I am not sure about this one :-)) the provided password or the username.

The api.py code features the following:
def power_on(self, system, user=None, password=None, logger=None):
"""
Powers up a system that has power management configured.
"""
return action_power.PowerTool(self._config,system,self,user,password,logger=logger).power("on")

and in action_power.py the following code is found under the 'power' method

def power(self, desired_state):
  ...
  template = self.get_command_template()
    template_file = open(template, "r")

    meta = utils.blender(self.api, False, self.system)
    meta["power_mode"] = desired_state

    # allow command line overrides of the username/password
    if self.force_user is not None:
       meta["power_user"] = self.force_user
    if self.force_pass is not None:
       meta["power_pass"] = self.force_pass

    tmp = templar.Templar(self.api._config)
    cmd = tmp.render(template_file, meta, None, self.system)
    template_file.close()

    cmd = cmd.strip()
    ...
    # use shell so we can have mutliple power commands chained together
    cmd = ['/bin/sh','-c', cmd]

    # Try the power command 5 times before giving up.
    # Some power switches are flakey
    for x in range(0,5):
        output, rc = utils.subprocess_sp(self.logger, cmd, shell=False)
  see [0] for some of the source code in the utils.subprocess_sp method.

while the shell=False is passed (eventually) through to the subprocess.Popen method, as the shell /bin/sh[1] has been provided in front of the command passed in shell meta-characters will be actually be a problem. As far as I can tell the template cmd rendering will not strip out shell meta-characters and opens up a command injection attack vector.

[0] utils.subprocess_sp(found in utils.py) just does the following -->
def subprocess_sp(logger, cmd, shell=True):
if logger is not None:
logger.info("running: %s" % cmd)
try:
sp = sub_process.Popen(cmd, shell=shell, stdout=sub_process.PIPE, stderr=sub_process.PIPE, close_fds=True)
...

[1] To verify that this is the case you test it out -->

import subprocess
subprocess.Popen(["/bin/sh", "-c", "echo lol && /bin/sh"], shell=False)
<subprocess.Popen object at 0x7f0ecaa92b50>
lol
sh-4.1$"

Comment #1:
"Without hitting the xmlrpc directly(just using a local python test script) I was able to inject shell commands by providing the following user-name:

";my command goes here ;" (when the power_rsa.template file was the selected power template)."

The Launchpad bug is currently private. I would be happy to give you access if you have a Launchpad account.

@jimi-c
Copy link
Member

jimi-c commented Apr 20, 2012

On Fri, Apr 20, 2012 at 9:28 AM, jdstrand
reply@reply.github.com
wrote:

This is forwarded from https://launchpad.net/bugs/978999:

"It appears as if the power_system method exposed in the xmlrpc api is vulnerable to command injection through either the system handle(I am not sure about this one :-)) the provided password or the username.

This is an overall issue with Popen, and not something that's easy to
fix. There was an attempt to mitigate it in cobbler but the && (and ||
I'm sure) aren't in the list of things it removes from the
username/password fields.

The solution I proposed on the dev list a while back was to send that
data to the command via STDIN instead of on the command line (more
secure anyway if someone's watching process lists). I have a branch
locally that started fleshing this out, but the main issue will be to
get people to switch their templates out.

The other possible solution is for people to change all of their power
templates, which use double quotes around variables. If they used
single quotes, shell characters would not be interpreted as you show.

@d1b
Copy link

d1b commented Apr 21, 2012

Blacklisting what username and passwords can be provided via the API is not a good solution.
Communicating over STDIN is a much better solution.

@mpdehaan
Copy link
Contributor

Obviously this is an undesirable thing. That being said, a cobbler API handle can basically control what every machine on your network is going to install. It should be handed out with the same level of concern as the root password to the cobbler server. It's basically root equivalent because it has to be. That being said, this issue should still be addressed, but it's not a world ending problem considering what the API handle is generally capable of anyway.

@jimi-c
Copy link
Member

jimi-c commented Apr 22, 2012

I believe this is ready to be tested, but since it's a pretty major change I'm not going to merge it in quite yet:

https://github.com/jimi1283/cobbler/tree/powerpipe

I've also started trying to address issue #71 with this, though I'm not sure how consistent the output from the status commands will be so I'm not sure exactly how to address the issue completely.

@jimi-c
Copy link
Member

jimi-c commented May 7, 2012

Closing this as the new code has been merged into the master branch and should be available in the next major release.

@jimi-c jimi-c closed this as completed May 7, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Report Reporting a bug Security
Projects
None yet
Development

No branches or pull requests

4 participants