From f02cd7ddb6cc5ae3c9fe5d1b78abcff471ffc461 Mon Sep 17 00:00:00 2001 From: Nate Coraor Date: Tue, 12 Sep 2017 15:06:30 -0400 Subject: [PATCH] Security: Fix issues with path handling in libraries and in general. --- config/galaxy.ini.sample | 8 + lib/galaxy/config.py | 1 + lib/galaxy/managers/folders.py | 4 +- lib/galaxy/managers/libraries.py | 2 +- lib/galaxy/tools/parameters/grouping.py | 2 + lib/galaxy/util/__init__.py | 48 ++--- lib/galaxy/util/path/__init__.py | 185 ++++++++++++++++++ lib/galaxy/util/path/ntpath.py | 11 ++ lib/galaxy/util/path/posixpath.py | 11 ++ .../webapps/galaxy/api/library_datasets.py | 33 +++- lib/galaxy/webapps/galaxy/api/remote_files.py | 4 +- .../webapps/galaxy/controllers/library.py | 2 + .../galaxy/controllers/library_common.py | 47 ++++- lib/tool_shed/managers/groups.py | 2 +- 14 files changed, 303 insertions(+), 57 deletions(-) create mode 100644 lib/galaxy/util/path/__init__.py create mode 100644 lib/galaxy/util/path/ntpath.py create mode 100644 lib/galaxy/util/path/posixpath.py diff --git a/config/galaxy.ini.sample b/config/galaxy.ini.sample index a72084bcb7a2..77b673ffbe5e 100644 --- a/config/galaxy.ini.sample +++ b/config/galaxy.ini.sample @@ -911,6 +911,14 @@ use_interactive = True # sub-directories of files contained in their directory. #user_library_import_dir = None +# For security reasons, users may not import any files that actually lie +# outside of their `user_library_import_dir` (e.g. using symbolic links). A +# list of directories can be allowed by setting the following option (the list +# is comma-separated). Be aware that *any* user with library import permissions +# can import from anywhere in these directories (assuming they are able to +# create symlinks to them). +#user_library_import_symlink_whitelist = None + # Allow admins to paste filesystem paths during upload. For libraries this # adds an option to the admin library upload tool allowing admins to paste # filesystem paths to files and directories in a box, and these paths will be diff --git a/lib/galaxy/config.py b/lib/galaxy/config.py index 756cc8f0a133..4514fdbed9f4 100644 --- a/lib/galaxy/config.py +++ b/lib/galaxy/config.py @@ -394,6 +394,7 @@ def __init__(self, **kwargs): self.genomespace_ui_url = kwargs.get('genomespace_ui_url', 'https://gsui.genomespace.org/jsui/') self.library_import_dir = kwargs.get('library_import_dir', None) self.user_library_import_dir = kwargs.get('user_library_import_dir', None) + self.user_library_import_symlink_whitelist = listify(kwargs.get('user_library_import_symlink_whitelist', []), do_strip=True) # Searching data libraries self.enable_lucene_library_search = string_as_bool(kwargs.get('enable_lucene_library_search', False)) self.enable_whoosh_library_search = string_as_bool(kwargs.get('enable_whoosh_library_search', False)) diff --git a/lib/galaxy/managers/folders.py b/lib/galaxy/managers/folders.py index d681e6a78da3..506518b0b2eb 100644 --- a/lib/galaxy/managers/folders.py +++ b/lib/galaxy/managers/folders.py @@ -132,7 +132,7 @@ def create(self, trans, parent_folder_id, new_folder_name, new_folder_descriptio """ parent_folder = self.get(trans, parent_folder_id) current_user_roles = trans.get_current_user_roles() - if not (trans.user_is_admin or trans.app.security_agent.can_add_library_item(current_user_roles, parent_folder)): + if not (trans.user_is_admin() or trans.app.security_agent.can_add_library_item(current_user_roles, parent_folder)): raise InsufficientPermissionsException('You do not have proper permission to create folders under given folder.') new_folder = trans.app.model.LibraryFolder(name=new_folder_name, description=new_folder_description) # We are associating the last used genome build with folders, so we will always @@ -230,7 +230,7 @@ def can_add_item(self, trans, folder): """ Return true if the user has permissions to add item to the given folder. """ - if trans.user_is_admin: + if trans.user_is_admin(): return True current_user_roles = trans.get_current_user_roles() add_roles = set(trans.app.security_agent.get_roles_for_action(folder, trans.app.security_agent.permitted_actions.LIBRARY_ADD)) diff --git a/lib/galaxy/managers/libraries.py b/lib/galaxy/managers/libraries.py index b94217ee53ff..ed7ef58c39b0 100644 --- a/lib/galaxy/managers/libraries.py +++ b/lib/galaxy/managers/libraries.py @@ -50,7 +50,7 @@ def create(self, trans, name, description='', synopsis=''): """ Create a new library. """ - if not trans.user_is_admin: + if not trans.user_is_admin(): raise exceptions.ItemAccessibilityException('Only administrators can create libraries.') else: library = trans.app.model.Library(name=name, description=description, synopsis=synopsis) diff --git a/lib/galaxy/tools/parameters/grouping.py b/lib/galaxy/tools/parameters/grouping.py index 1531bcef5980..85558f2a7de2 100644 --- a/lib/galaxy/tools/parameters/grouping.py +++ b/lib/galaxy/tools/parameters/grouping.py @@ -380,6 +380,7 @@ def get_one_filename(context): warnings.append("All FTP uploaded file selections were ignored.") elif ftp_files is not None and trans.user is not None: # look for files uploaded via FTP user_ftp_dir = trans.user_ftp_dir + assert not os.path.islink(user_ftp_dir), "User FTP directory cannot be a symbolic link" for (dirpath, dirnames, filenames) in os.walk(user_ftp_dir): for filename in filenames: for ftp_filename in ftp_files: @@ -449,6 +450,7 @@ def get_filenames(context): # TODO: warning to the user (could happen if session has become invalid) else: user_ftp_dir = trans.user_ftp_dir + assert not os.path.islink(user_ftp_dir), "User FTP directory cannot be a symbolic link" for (dirpath, dirnames, filenames) in os.walk(user_ftp_dir): for filename in filenames: path = relpath(os.path.join(dirpath, filename), user_ftp_dir) diff --git a/lib/galaxy/util/__init__.py b/lib/galaxy/util/__init__.py index e3f2eb6e4a51..062bc4b216f1 100644 --- a/lib/galaxy/util/__init__.py +++ b/lib/galaxy/util/__init__.py @@ -8,6 +8,7 @@ import binascii import collections import errno +import importlib import json import logging import os @@ -29,7 +30,7 @@ from datetime import datetime from hashlib import md5 -from os.path import normpath, relpath +from os.path import relpath from xml.etree import ElementInclude, ElementTree from xml.etree.ElementTree import ParseError @@ -49,6 +50,7 @@ docutils_html4css1 = None from .inflection import English, Inflector +from .path import safe_contains, safe_makedirs, safe_relpath # noqa: F401 inflector = Inflector(English) @@ -604,31 +606,19 @@ def which(file): return None -def safe_makedirs(path): - """ Safely make a directory, do not fail if it already exist or - is created during execution. - """ - if not os.path.exists(path): - try: - os.makedirs(path) - except OSError as e: - # review source for Python 2.7 this would only ever happen - # for the last path anyway so need to recurse - this exception - # means the last part of the path was already in existence. - if e.errno != errno.EEXIST: - raise - - def in_directory(file, directory, local_path_module=os.path): """ Return true, if the common prefix of both is equal to directory e.g. /a/b/c/d.rst and directory is /a/b, the common prefix is /a/b - """ - # Make both absolute. - directory = local_path_module.abspath(directory) - file = local_path_module.abspath(file) - return local_path_module.commonprefix([file, directory]) == directory + local_path_module is used by Pulsar to check Windows paths while running on + a POSIX-like system. + """ + if local_path_module != os.path: + _safe_contains = importlib.import_module('galaxy.util.path.%s' % local_path_module.__name__).safe_contains + else: + _safe_contains = safe_contains + return _safe_contains(directory, file) def merge_sorted_iterables(operator, *iterables): @@ -1505,22 +1495,6 @@ def download_to_file(url, dest_file_path, timeout=30, chunk_size=2 ** 20): f.write(chunk) -def safe_relpath(path): - """ - Given what we expect to be a relative path, determine whether the path - would exist inside the current directory. - - :type path: string - :param path: a path to check - :rtype: bool - :returns: ``True`` if path is relative and does not reference a path - in a parent directory, ``False`` otherwise. - """ - if path.startswith(os.sep) or normpath(path).startswith(os.pardir): - return False - return True - - class ExecutionTimer(object): def __init__(self): diff --git a/lib/galaxy/util/path/__init__.py b/lib/galaxy/util/path/__init__.py new file mode 100644 index 000000000000..48a8355ad79e --- /dev/null +++ b/lib/galaxy/util/path/__init__.py @@ -0,0 +1,185 @@ +"""Path manipulation functions. +""" +from __future__ import absolute_import + +import errno +import imp +from functools import partial +from itertools import starmap +from operator import getitem +from os import ( + makedirs, + walk, +) +from os.path import ( + abspath, + exists, + isabs, + join, + normpath, + pardir, + realpath, + relpath, +) + +from six import string_types +from six.moves import filterfalse, map, zip + + +def safe_contains(prefix, path, whitelist=None): + """Ensure a path is contained within another path. + + Given any two filesystem paths, ensure that ``path`` is contained in ``prefix``. If ``path`` exists (either as an + absolute path or relative to ``prefix``), it is canonicalized with :func:`os.path.realpath` to ensure it is not a + symbolic link that points outside of ``path``. If it is a symbolic link and ``whitelist`` is set, the symbolic link + may also point inside a ``whitelist`` path. + + The ``path`` is checked against ``whitelist`` using either its absolute pathname (if passed in as absolute) or + relative to ``prefix`` and canonicalized (if applicable). It is *not* ``os.path.join()``ed with each ``whitelist`` + directory. + + :type prefix: string + :param prefix: a directory under which ``path`` is to be checked + :type path: string + :param path: a filename to check + :type whitelist: list of strings + :param whitelist: list of additional paths under which ``path`` may be located + :rtype: bool + :returns: ``True`` if ``path`` is contained within ``prefix`` or ``whitelist``, ``False`` otherwise. + """ + return any(__contains(prefix, path, whitelist=whitelist)) + + +def safe_makedirs(path): + """Safely make a directory, do not fail if it already exists or is created during execution. + + :type path: string + :param path: a directory to create + """ + # prechecking for existence is faster than try/except + if not exists(path): + try: + makedirs(path) + except OSError as e: + # reviewing the source for Python 2.7, this would only ever happen for the last path element anyway so no + # need to recurse - this exception means the last part of the path was already in existence. + if e.errno != errno.EEXIST: + raise + + +def safe_relpath(path): + """Determine whether a relative path references a path outside its root. + + This is a path computation: the filesystem is not accessed to confirm the existence or nature of ``path``. + + :type path: string + :param path: a path to check + :rtype: bool + :returns: ``True`` if path is relative and does not reference a path in a parent directory, ``False`` + otherwise. + """ + return not (isabs(path) or normpath(path).startswith(pardir)) + + +def unsafe_walk(path, whitelist=None): + """Walk a path and ensure that none of its contents are symlinks outside the path. + + It is assumed that ``path`` itself has already been validated e.g. with :func:`safe_relpath` or + :func:`safe_contains`. + + :type path: string + :param path: a directory to check for unsafe contents + :type whitelist: list of strings + :param whitelist: list of additional paths under which contents may be located + :rtype: iterator + :returns: Iterator of "bad" files found under ``path`` + """ + return filterfalse(partial(safe_contains, path, whitelist=whitelist), __walk(abspath(path))) + + +def __listify(item): + """A non-splitting version of :func:`galaxy.util.listify`. + """ + if not item: + return [] + elif isinstance(item, list) or isinstance(item, tuple): + return item + else: + return [item] + + +# helpers + + +def __walk(path): + for dirpath, dirnames, filenames in walk(path): + for name in dirnames + filenames: + yield join(dirpath, name) + + +def __contains(prefix, path, whitelist=None): + real = realpath(join(prefix, path)) + yield not relpath(real, prefix).startswith(pardir) + for wldir in whitelist or []: + yield not relpath(real, wldir).startswith(pardir) + + +# cross-platform support + + +def _build_self(target, path_module): + """Populate a module with the same exported functions as this module, but using the given os.path module. + + :type target: module + :param target: module on which to set ``galaxy.util.path`` functions + :type path_module: ``ntpath`` or ``posixpath`` module + :param path_module: module implementing ``os.path`` API to use for path functions + """ + __copy_self().__set_fxns_on(target, path_module) + + +def __copy_self(names=__name__, parent=None): + """Returns a copy of this module that can be modified without modifying `galaxy.util.path`` in ``sys.modules``. + """ + if isinstance(names, string_types): + names = iter(names.split('.')) + try: + name = names.next() + except StopIteration: + return parent + path = parent and parent.__path__ + parent = imp.load_module(name, *imp.find_module(name, path)) + return __copy_self(names, parent) + + +def __set_fxns_on(target, path_module): + """Overrides imported os.path functions with the ones from path_module and populates target with the global + functions from this module. + """ + for name in __pathfxns__: + globals()[name] = getattr(path_module, name) + __get = partial(getitem, globals()) + __set = partial(setattr, target) + # this is actually izip(..., imap(...)) + __fxns = zip(__all__, map(__get, __all__)) + # list() to execute + list(starmap(__set, __fxns)) + + +__pathfxns__ = ( + 'abspath', + 'exists', + 'isabs', + 'join', + 'normpath', + 'pardir', + 'realpath', + 'relpath', +) + +__all__ = ( + 'safe_contains', + 'safe_makedirs', + 'safe_relpath', + 'unsafe_walk', +) diff --git a/lib/galaxy/util/path/ntpath.py b/lib/galaxy/util/path/ntpath.py new file mode 100644 index 000000000000..5363409b3af6 --- /dev/null +++ b/lib/galaxy/util/path/ntpath.py @@ -0,0 +1,11 @@ +"""Galaxy "safe" path functions forced to work with Windows-style paths regardless of current platform +""" +from __future__ import absolute_import + +import ntpath +import sys + +from . import _build_self + + +_build_self(sys.modules[__name__], ntpath) diff --git a/lib/galaxy/util/path/posixpath.py b/lib/galaxy/util/path/posixpath.py new file mode 100644 index 000000000000..28232649b57d --- /dev/null +++ b/lib/galaxy/util/path/posixpath.py @@ -0,0 +1,11 @@ +"""Galaxy "safe" path functions forced to work with POSIX-style paths regardless of current platform +""" +from __future__ import absolute_import + +import posixpath +import sys + +from . import _build_self + + +_build_self(sys.modules[__name__], posixpath) diff --git a/lib/galaxy/webapps/galaxy/api/library_datasets.py b/lib/galaxy/webapps/galaxy/api/library_datasets.py index b2030e0038e4..0b1ba0c0c3a8 100644 --- a/lib/galaxy/webapps/galaxy/api/library_datasets.py +++ b/lib/galaxy/webapps/galaxy/api/library_datasets.py @@ -18,6 +18,7 @@ from galaxy.managers import folders, library_datasets, roles from galaxy.tools.actions import upload_common from galaxy.tools.parameters import populate_state +from galaxy.util.path import safe_contains, safe_relpath, unsafe_walk from galaxy.util.streamball import StreamBall from galaxy.web import _future_expose_api as expose_api from galaxy.web import _future_expose_api_anonymous as expose_api_anonymous @@ -408,29 +409,45 @@ def load(self, trans, payload=None, **kwd): source = kwd.get('source', None) if source not in ['userdir_file', 'userdir_folder', 'importdir_file', 'importdir_folder', 'admin_path']: raise exceptions.RequestParameterMissingException('You have to specify "source" parameter. Possible values are "userdir_file", "userdir_folder", "admin_path", "importdir_file" and "importdir_folder". ') - if source in ['importdir_file', 'importdir_folder']: - if not trans.user_is_admin: + elif source in ['importdir_file', 'importdir_folder']: + if not trans.user_is_admin(): raise exceptions.AdminRequiredException('Only admins can import from importdir.') if not trans.app.config.library_import_dir: raise exceptions.ConfigDoesNotAllowException('The configuration of this Galaxy instance does not allow admins to import into library from importdir.') import_base_dir = trans.app.config.library_import_dir + if not safe_relpath(path): + # admins shouldn't be able to explicitly specify a path outside server_dir, but symlinks are allowed. + # the reasoning here is that galaxy admins may not have direct filesystem access or can only access + # library_import_dir via FTP (which cannot create symlinks), and may rely on sysadmins to set up the + # import directory. if they have filesystem access, all bets are off. + raise exceptions.RequestParameterInvalidException( 'The given path is invalid.' ) path = os.path.join(import_base_dir, path) - if source in ['userdir_file', 'userdir_folder']: + elif source in ['userdir_file', 'userdir_folder']: + unsafe = None user_login = trans.user.email user_base_dir = trans.app.config.user_library_import_dir if user_base_dir is None: raise exceptions.ConfigDoesNotAllowException('The configuration of this Galaxy instance does not allow upload from user directories.') full_dir = os.path.join(user_base_dir, user_login) - if not path.lower().startswith(full_dir.lower()): + if not safe_contains(full_dir, path, whitelist=trans.app.config.user_library_import_symlink_whitelist): + # the path is a symlink outside the user dir path = os.path.join(full_dir, path) + log.error('User attempted to import a path that resolves to a path outside of their import dir: %s -> %s', path, os.path.realpath(path)) + raise exceptions.RequestParameterInvalidException('The given path is invalid.') + path = os.path.join(full_dir, path) + for unsafe in unsafe_walk(path, whitelist=[full_dir] + trans.app.config.user_library_import_symlink_whitelist): + # the path is a dir and contains files that symlink outside the user dir + log.error('User attempted to import a directory containing a path that resolves to a path outside of their import dir: %s -> %s', unsafe, os.path.realpath(unsafe)) + if unsafe: + raise exceptions.RequestParameterInvalidException('The given path is invalid.') if not os.path.exists(path): raise exceptions.RequestParameterInvalidException('Given path does not exist on the host.') if not self.folder_manager.can_add_item(trans, folder): raise exceptions.InsufficientPermissionsException('You do not have proper permission to add items to the given folder.') - if source == 'admin_path': + elif source == 'admin_path': if not trans.app.config.allow_library_path_paste: raise exceptions.ConfigDoesNotAllowException('The configuration of this Galaxy instance does not allow admins to import into library from path.') - if not trans.user_is_admin: + if not trans.user_is_admin(): raise exceptions.AdminRequiredException('Only admins can import from path.') # Set up the traditional tool state/params @@ -449,12 +466,12 @@ def load(self, trans, payload=None, **kwd): if source in ['importdir_folder']: kwd['filesystem_paths'] = os.path.join(import_base_dir, path) # user wants to import one file only - if source in ["userdir_file", "importdir_file"]: + elif source in ["userdir_file", "importdir_file"]: file = os.path.abspath(path) abspath_datasets.append(trans.webapp.controllers['library_common'].make_library_uploaded_dataset( trans, 'api', kwd, os.path.basename(file), file, 'server_dir', library_bunch)) # user wants to import whole folder - if source == "userdir_folder": + elif source == "userdir_folder": uploaded_datasets_bunch = trans.webapp.controllers['library_common'].get_path_paste_uploaded_datasets( trans, 'api', kwd, library_bunch, 200, '') uploaded_datasets = uploaded_datasets_bunch[0] diff --git a/lib/galaxy/webapps/galaxy/api/remote_files.py b/lib/galaxy/webapps/galaxy/api/remote_files.py index 9dc39fc8222f..7360f8ec12d8 100644 --- a/lib/galaxy/webapps/galaxy/api/remote_files.py +++ b/lib/galaxy/webapps/galaxy/api/remote_files.py @@ -107,7 +107,7 @@ def __load_all_filenames(self, directory): subfolders and returns a flat list. """ response = [] - if os.path.exists(directory): + if os.path.exists(directory) and not os.path.islink(directory): for (dirpath, dirnames, filenames) in os.walk(directory): for filename in filenames: path = os.path.relpath(os.path.join(dirpath, filename), directory) @@ -130,7 +130,7 @@ def __create_jstree(self, directory, disable='folders'): """ userdir_jstree = None jstree_paths = [] - if os.path.exists(directory): + if os.path.exists(directory) and not os.path.islink(directory): for (dirpath, dirnames, filenames) in os.walk(directory): for dirname in dirnames: dir_path = os.path.relpath(os.path.join(dirpath, dirname), directory) diff --git a/lib/galaxy/webapps/galaxy/controllers/library.py b/lib/galaxy/webapps/galaxy/controllers/library.py index 7dbac8ca5611..dacb9e8c6bfb 100644 --- a/lib/galaxy/webapps/galaxy/controllers/library.py +++ b/lib/galaxy/webapps/galaxy/controllers/library.py @@ -91,6 +91,7 @@ def list(self, trans, **kwd): 'app': app}) @web.expose + @web.require_admin def index(self, trans, **kwd): message = escape(kwd.get('message', '')) status = escape(kwd.get('status', 'done')) @@ -101,6 +102,7 @@ def index(self, trans, **kwd): status=status) @web.expose + @web.require_admin def browse_libraries(self, trans, **kwd): if 'operation' in kwd: operation = kwd['operation'].lower() diff --git a/lib/galaxy/webapps/galaxy/controllers/library_common.py b/lib/galaxy/webapps/galaxy/controllers/library_common.py index 955fede10f0b..44ad3e2e4ec9 100644 --- a/lib/galaxy/webapps/galaxy/controllers/library_common.py +++ b/lib/galaxy/webapps/galaxy/controllers/library_common.py @@ -23,6 +23,7 @@ from galaxy.tools.actions import upload_common from galaxy.tools.parameters import populate_state from galaxy.util import inflector, unicodify, FILENAME_VALID_CHARS +from galaxy.util.path import safe_contains, safe_relpath, unsafe_walk from galaxy.util.streamball import StreamBall from galaxy.web.base.controller import BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMetadataMixin, UsesLibraryMixinItems from galaxy.web.form_builder import AddressField, CheckboxField, SelectField, build_select_field @@ -72,6 +73,7 @@ class LibraryCommon(BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMetadataMixin, UsesLibraryMixinItems): @web.json + @web.require_admin def library_item_updates(self, trans, ids=None, states=None): # Avoid caching trans.response.headers['Pragma'] = 'no-cache' @@ -95,6 +97,7 @@ def library_item_updates(self, trans, ids=None, states=None): return rval @web.expose + @web.require_admin def browse_library(self, trans, cntrller='library', **kwd): message = escape(kwd.get('message', '')) status = kwd.get('status', 'done') @@ -167,6 +170,7 @@ def browse_library(self, trans, cntrller='library', **kwd): status=status)) @web.expose + @web.require_admin def library_info(self, trans, cntrller, **kwd): message = escape(kwd.get('message', '')) status = kwd.get('status', 'done') @@ -226,6 +230,7 @@ def library_info(self, trans, cntrller, **kwd): status=escape(status)) @web.expose + @web.require_admin def library_permissions(self, trans, cntrller, **kwd): message = escape(kwd.get('message', '')) status = kwd.get('status', 'done') @@ -273,6 +278,7 @@ def library_permissions(self, trans, cntrller, **kwd): status=escape(status)) @web.expose + @web.require_admin def create_folder(self, trans, cntrller, parent_id, library_id, **kwd): message = escape(kwd.get('message', '')) status = kwd.get('status', 'done') @@ -348,6 +354,7 @@ def create_folder(self, trans, cntrller, parent_id, library_id, **kwd): status=escape(status)) @web.expose + @web.require_admin def folder_info(self, trans, cntrller, id, library_id, **kwd): message = escape(kwd.get('message', '')) status = kwd.get('status', 'done') @@ -404,6 +411,7 @@ def folder_info(self, trans, cntrller, id, library_id, **kwd): status=escape(status)) @web.expose + @web.require_admin def folder_permissions(self, trans, cntrller, id, library_id, **kwd): message = escape(kwd.get('message', '')) status = kwd.get('status', 'done') @@ -454,6 +462,7 @@ def folder_permissions(self, trans, cntrller, id, library_id, **kwd): status=escape(status)) @web.expose + @web.require_admin def ldda_edit_info(self, trans, cntrller, library_id, folder_id, id, **kwd): message = escape(kwd.get('message', '')) status = kwd.get('status', 'done') @@ -605,6 +614,7 @@ def __ok_to_edit_metadata(ldda_id): status=escape(status)) @web.expose + @web.require_admin def ldda_info(self, trans, cntrller, library_id, folder_id, id, **kwd): message = escape(kwd.get('message', '')) status = kwd.get('status', 'done') @@ -654,6 +664,7 @@ def ldda_info(self, trans, cntrller, library_id, folder_id, id, **kwd): status=escape(status)) @web.expose + @web.require_admin def ldda_permissions(self, trans, cntrller, library_id, folder_id, id, **kwd): message = str(escape(kwd.get('message', ''))) status = kwd.get('status', 'done') @@ -792,6 +803,7 @@ def ldda_permissions(self, trans, cntrller, library_id, folder_id, id, **kwd): status=escape(status)) @web.expose + @web.require_admin def upload_library_dataset(self, trans, cntrller, library_id, folder_id, **kwd): message = escape(kwd.get('message', '')) status = kwd.get('status', 'done') @@ -851,6 +863,9 @@ def upload_library_dataset(self, trans, cntrller, library_id, folder_id, **kwd): elif upload_option == 'upload_paths' and not is_admin: error = True message = 'Uploading files via filesystem paths can only be performed by administrators' + elif upload_option not in ('upload_file', 'upload_directory', 'upload_paths'): + error = True + message = 'Invalid upload_option' elif roles: # Check to see if the user selected roles to associate with the DATASET_ACCESS permission # on the dataset that would cause accessibility issues. @@ -1066,17 +1081,26 @@ def upload_dataset(self, trans, cntrller, library_id, folder_id, replace_dataset if upload_option == 'upload_directory': if server_dir in [None, 'None', '']: response_code = 400 - if cntrller == 'library_admin' or (cntrller == 'api' and trans.user_is_admin): + if trans.user_is_admin() and cntrller in ('library_admin', 'api'): import_dir = trans.app.config.library_import_dir import_dir_desc = 'library_import_dir' - full_dir = os.path.join(import_dir, server_dir) else: import_dir = trans.app.config.user_library_import_dir + if server_dir != trans.user.email: + import_dir = os.path.join(import_dir, trans.user.email) import_dir_desc = 'user_library_import_dir' - if server_dir == trans.user.email: - full_dir = os.path.join(import_dir, server_dir) - else: - full_dir = os.path.join(import_dir, trans.user.email, server_dir) + full_dir = os.path.join(import_dir, server_dir) + unsafe = None + if safe_relpath(server_dir): + if import_dir_desc == 'user_library_import_dir' and safe_contains(import_dir, full_dir, whitelist=trans.app.config.user_library_import_symlink_whitelist): + for unsafe in unsafe_walk(full_dir, whitelist=[import_dir] + trans.app.config.user_library_import_symlink_whitelist): + log.error('User attempted to import a path that resolves to a path outside of their import dir: %s -> %s', unsafe, os.path.realpath(unsafe)) + else: + log.error('User attempted to import a directory path that resolves to a path outside of their import dir: %s -> %s', server_dir, os.path.realpath(full_dir)) + unsafe = True + if unsafe: + response_code = 403 + message = 'Invalid server_dir' if import_dir: message = 'Select a directory' else: @@ -1270,6 +1294,7 @@ def _check_path_paste_params(self, params): return None @web.expose + @web.require_admin def add_history_datasets_to_library(self, trans, cntrller, library_id, folder_id, hda_ids='', **kwd): message = escape(kwd.get('message', '')) status = kwd.get('status', 'done') @@ -1512,6 +1537,7 @@ def _build_upload_option_select_list(self, trans, upload_option, is_admin, do_no return upload_option_select_list @web.expose + @web.require_admin def download_dataset_from_folder(self, trans, cntrller, id, library_id=None, **kwd): """Catches the dataset id and displays file contents as directed""" show_deleted = util.string_as_bool(kwd.get('show_deleted', False)) @@ -1551,6 +1577,7 @@ def download_dataset_from_folder(self, trans, cntrller, id, library_id=None, **k status='error')) @web.expose + @web.require_admin def library_dataset_info(self, trans, cntrller, id, library_id, **kwd): message = escape(kwd.get('message', '')) status = kwd.get('status', 'done') @@ -1599,6 +1626,7 @@ def library_dataset_info(self, trans, cntrller, id, library_id, **kwd): status=escape(status)) @web.expose + @web.require_admin def library_dataset_permissions(self, trans, cntrller, id, library_id, **kwd): message = escape(kwd.get('message', '')) status = kwd.get('status', 'done') @@ -1647,6 +1675,7 @@ def library_dataset_permissions(self, trans, cntrller, id, library_id, **kwd): status=escape(status)) @web.expose + @web.require_admin def make_library_item_public(self, trans, cntrller, library_id, item_type, id, **kwd): message = escape(kwd.get('message', '')) status = kwd.get('status', 'done') @@ -1689,6 +1718,7 @@ def make_library_item_public(self, trans, cntrller, library_id, item_type, id, * status=status)) @web.expose + @web.require_admin def act_on_multiple_datasets(self, trans, cntrller, library_id=None, ldda_ids='', **kwd): # This method is called from 1 of 3 places: # - this controller's download_dataset_from_folder() method @@ -1981,6 +2011,7 @@ def __str__(self): status=escape(status)) @web.expose + @web.require_admin def import_datasets_to_histories(self, trans, cntrller, library_id='', folder_id='', ldda_ids='', target_history_id='', target_history_ids='', new_history_name='', **kwd): # This method is called from one of the following places: # - a menu option for a library dataset ( ldda_ids is a single ldda id ) @@ -2111,6 +2142,7 @@ def import_datasets_to_histories(self, trans, cntrller, library_id='', folder_id status=escape(status)) @web.expose + @web.require_admin def manage_template_inheritance(self, trans, cntrller, item_type, library_id, folder_id=None, ldda_id=None, **kwd): show_deleted = util.string_as_bool(kwd.get('show_deleted', False)) use_panels = util.string_as_bool(kwd.get('use_panels', False)) @@ -2156,6 +2188,7 @@ def manage_template_inheritance(self, trans, cntrller, item_type, library_id, fo status='done')) @web.expose + @web.require_admin def move_library_item(self, trans, cntrller, item_type, item_id, source_library_id='', make_target_current=True, **kwd): # This method is called from one of the following places: # - a menu option for a library dataset ( item_type is 'ldda' and item_id is a single ldda id ) @@ -2373,6 +2406,7 @@ def __build_target_folder_id_select_field(trans, folders, selected_value='none') status=escape(status)) @web.expose + @web.require_admin def delete_library_item(self, trans, cntrller, library_id, item_id, item_type, **kwd): # This action will handle deleting all types of library items. State is saved for libraries and # folders ( i.e., if undeleted, the state of contents of the library or folder will remain, so previously @@ -2441,6 +2475,7 @@ def delete_library_item(self, trans, cntrller, library_id, item_id, item_type, * status=status)) @web.expose + @web.require_admin def undelete_library_item(self, trans, cntrller, library_id, item_id, item_type, **kwd): # This action will handle undeleting all types of library items status = kwd.get('status', 'done') diff --git a/lib/tool_shed/managers/groups.py b/lib/tool_shed/managers/groups.py index f590850aeed9..117a192169ac 100644 --- a/lib/tool_shed/managers/groups.py +++ b/lib/tool_shed/managers/groups.py @@ -56,7 +56,7 @@ def create(self, trans, name, description=''): """ Create a new group. """ - if not trans.user_is_admin: + if not trans.user_is_admin(): raise ItemAccessibilityException('Only administrators can create groups.') else: if self.get(trans, name=name):