Skip to content
Permalink
Browse files

Security: Switch GIE Popen() calls to run without shell=True to fix an

ACE vulnerability.
  • Loading branch information...
natefoo committed Sep 29, 2017
1 parent e8a7225 commit 9f8d3ee444ad10038add204ed1c1dc11e636dd9d
Showing with 36 additions and 34 deletions.
  1. +36 −34 lib/galaxy/web/base/interactive_environments.py
@@ -3,14 +3,17 @@
import logging
import os
import random
import shlex
import stat
import tempfile
import uuid
from itertools import product

from subprocess import PIPE, Popen
from sys import platform as _platform

import yaml
from six.moves import shlex_quote

from galaxy import model, web
from galaxy.containers import build_container_interfaces
@@ -126,7 +129,7 @@ def load_deploy_config(self, default_dict={}):
# their defaults dictionary instead.
default_dict = {
'container_interface': None,
'command': 'docker {docker_args}',
'command': 'docker',
'command_inject': '-e DEBUG=false -e DEFAULT_CONTAINER_RUNTIME=120',
'docker_hostname': 'localhost',
'wx_tempdir': 'False',
@@ -263,40 +266,40 @@ def _get_import_volume_for_run(self):
def _get_name_for_run(self):
return CONTAINER_NAME_PREFIX + uuid.uuid4().hex

def base_docker_cmd(self, subcmd=None):
# This is the basic docker command such as "sudo -u docker docker" or just "docker"
# Previously, {docker_args} was required to be in the string, this is no longer the case
base = shlex.split(self.attr.viz_config.get("docker", "command").format(docker_args='').strip())
if subcmd:
base.append(subcmd)
return base

def docker_cmd(self, image, env_override=None, volumes=None):
"""
Generate and return the docker command to execute
"""
if volumes is None:
volumes = []
env = self._get_env_for_run(env_override)
import_volume_def = self._get_import_volume_for_run()
env_str = ' '.join(['-e "%s=%s"' % (key, item) for key, item in env.items()])
volume_str = ' '.join(['-v "%s"' % volume for volume in volumes]) if self.use_volumes else ''
import_volume_str = '-v "{import_volume}"'.format(import_volume=import_volume_def) if import_volume_def else ''
name = None
# This is the basic docker command such as "sudo -u docker docker {docker_args}"
# or just "docker {docker_args}"
command = self.attr.viz_config.get("docker", "command")
# Then we format in the entire docker command in place of
# {docker_args}, so as to let the admin not worry about which args are
# getting passed
def _flag_opts(flag, opts):
return [arg for pair in product((flag,), opts) for arg in pair]

command_inject = self.attr.viz_config.get("docker", "command_inject")
# --name should really not be set, but we'll try to honor it anyway
if '--name' not in command_inject:
name = self._get_name_for_run()
command = command.format(docker_args='run {command_inject} {name} {environment} -d -P {import_volume_str} {volume_str} {image}')

# Once that's available, we format again with all of our arguments
command = command.format(
command_inject=command_inject,
name='--name=%s' % name if name is not None else '',
environment=env_str,
import_volume_str=import_volume_str,
volume_str=volume_str,
image=image,
name = ['--name=%s' % self._get_name_for_run()] if '--name' not in command_inject else []
env = self._get_env_for_run(env_override)
import_volume_def = self._get_import_volume_for_run()
if volumes is None:
volumes = []
if import_volume_def:
volumes.insert(0, import_volume_def)

return (
self.base_docker_cmd('run') +
shlex.split(command_inject) +
name +
_flag_opts('-e', ['='.join(map(str, t)) for t in env.items()]) +
['-d', '-P'] +
_flag_opts('-v', map(str, volumes)) +
[image]
)
return command

@property
def use_volumes(self):
@@ -368,9 +371,9 @@ def _launch_legacy(self, image, env_override, volumes):

log.info("Starting docker container for IE {0} with command [{1}]".format(
self.attr.viz_id,
raw_cmd
' '.join([shlex_quote(x) for x in raw_cmd])
))
p = Popen( raw_cmd, stdout=PIPE, stderr=PIPE, close_fds=True, shell=True)
p = Popen( raw_cmd, stdout=PIPE, stderr=PIPE, close_fds=True)
stdout, stderr = p.communicate()
if p.returncode != 0:
log.error( "%s\n%s" % (stdout, stderr) )
@@ -474,14 +477,13 @@ def inspect_container(self, container_id):
:returns: inspect_data, a dict of docker inspect output
"""
command = self.attr.viz_config.get("docker", "command")
command = command.format(docker_args="inspect %s" % container_id)
raw_cmd = self.base_docker_cmd('inspect') + [container_id]
log.info("Inspecting docker container {0} with command [{1}]".format(
container_id,
command
' '.join([shlex_quote(x) for x in raw_cmd])
))

p = Popen(command, stdout=PIPE, stderr=PIPE, close_fds=True, shell=True)
p = Popen(raw_cmd, stdout=PIPE, stderr=PIPE, close_fds=True)
stdout, stderr = p.communicate()
if p.returncode != 0:
log.error( "%s\n%s" % (stdout, stderr) )

0 comments on commit 9f8d3ee

Please sign in to comment.
You can’t perform that action at this time.