Security: Fix issues with path handling in libraries and in general.

config/galaxy.ini.sample

@@ -853,6 +853,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
+
 # Add 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
 # added to a library.  Set to True to enable.  Please note the security

lib/galaxy/config.py

@@ -288,6 +288,7 @@ def __init__( self, **kwargs ):
         self.screencasts_url = kwargs.get( 'screencasts_url', None )
         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 ) )

lib/galaxy/managers/folders.py

@@ -132,7 +132,7 @@ def create( self, trans, parent_folder_id, new_folder_name, new_folder_descripti
         """
         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 ) )

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 )

lib/galaxy/tools/parameters/grouping.py

@@ -355,6 +355,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:
@@ -418,6 +419,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 )

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 ):
@@ -1503,22 +1493,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):

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',
+)

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)

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)