From 93a8bfc7cb5e9c3395c5057910ec39d68ad787b4 Mon Sep 17 00:00:00 2001
From: Nate Coraor <nate@bx.psu.edu>
Date: Tue, 12 Sep 2017 15:06:30 -0400
Subject: [PATCH 1/9] 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 ++
 lib/galaxy/webapps/galaxy/api/lda_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/galaxy/webapps/galaxy/controllers/user.py |   2 +-
 lib/tool_shed/managers/groups.py              |   2 +-
 15 files changed, 304 insertions(+), 58 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 cc3f972e04ea..40b06aa5e4e3 100644
--- a/config/galaxy.ini.sample
+++ b/config/galaxy.ini.sample
@@ -801,6 +801,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
diff --git a/lib/galaxy/config.py b/lib/galaxy/config.py
index 5d67e3d78c86..a4c21887778c 100644
--- a/lib/galaxy/config.py
+++ b/lib/galaxy/config.py
@@ -285,6 +285,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 ) )
diff --git a/lib/galaxy/managers/folders.py b/lib/galaxy/managers/folders.py
index cbb4ad3e9b75..b835ae76e50d 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_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 ) )
diff --git a/lib/galaxy/managers/libraries.py b/lib/galaxy/managers/libraries.py
index 3fb32a2cb543..b4ca87538fe4 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 290534977f05..83e9d8ba6927 100644
--- a/lib/galaxy/tools/parameters/grouping.py
+++ b/lib/galaxy/tools/parameters/grouping.py
@@ -356,6 +356,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:
@@ -419,6 +420,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 707def64e843..e7914a6af35a 100644
--- a/lib/galaxy/util/__init__.py
+++ b/lib/galaxy/util/__init__.py
@@ -9,6 +9,7 @@
 import collections
 import errno
 import grp
+import importlib
 import json
 import logging
 import os
@@ -24,7 +25,7 @@
 import time
 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
 
@@ -41,6 +42,7 @@
     docutils_html4css1 = None
 
 from .inflection import English, Inflector
+from .path import safe_contains, safe_makedirs, safe_relpath  # noqa: F401
 
 inflector = Inflector(English)
 
@@ -600,31 +602,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 ):
@@ -1487,22 +1477,6 @@ def url_get( base_url, password_mgr=None, pathspec=None, params=None ):
     return content
 
 
-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/lda_datasets.py b/lib/galaxy/webapps/galaxy/api/lda_datasets.py
index 8e94edb29d20..05f9220bfedf 100644
--- a/lib/galaxy/webapps/galaxy/api/lda_datasets.py
+++ b/lib/galaxy/webapps/galaxy/api/lda_datasets.py
@@ -19,6 +19,7 @@
 from galaxy.exceptions import ObjectNotFound
 from galaxy.managers import folders, roles
 from galaxy.tools.actions import upload_common
+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
@@ -422,29 +423,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
@@ -463,12 +480,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 28d42cd45c02..904c186e2755 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 3df63fa6584a..245410bd6e54 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 6ecc6acba5df..da9e67b693c6 100644
--- a/lib/galaxy/webapps/galaxy/controllers/library_common.py
+++ b/lib/galaxy/webapps/galaxy/controllers/library_common.py
@@ -22,6 +22,7 @@
 from galaxy.security import Action
 from galaxy.tools.actions import upload_common
 from galaxy.util import inflector, unicodify
+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
@@ -71,6 +72,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'
@@ -94,6 +96,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' )
@@ -166,6 +169,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' )
@@ -225,6 +229,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' )
@@ -272,6 +277,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' )
@@ -347,6 +353,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' )
@@ -403,6 +410,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' )
@@ -453,6 +461,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' )
@@ -604,6 +613,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' )
@@ -653,6 +663,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' )
@@ -791,6 +802,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' )
@@ -846,6 +858,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.
@@ -1060,17 +1075,26 @@ def upload_dataset( self, trans, cntrller, library_id, folder_id, replace_datase
         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:
@@ -1262,6 +1286,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' )
@@ -1499,6 +1524,7 @@ def _build_upload_option_select_list( self, trans, upload_option, is_admin, do_n
         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 ) )
@@ -1539,6 +1565,7 @@ def download_dataset_from_folder( self, trans, cntrller, id, library_id=None, **
                                                           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' )
@@ -1587,6 +1614,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' )
@@ -1635,6 +1663,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' )
@@ -1677,6 +1706,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
@@ -1969,6 +1999,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 )
@@ -2099,6 +2130,7 @@ def import_datasets_to_histories( self, trans, cntrller, library_id='', folder_i
                                     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 ) )
@@ -2144,6 +2176,7 @@ def manage_template_inheritance( self, trans, cntrller, item_type, library_id, f
                                                           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 )
@@ -2361,6 +2394,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
@@ -2429,6 +2463,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/galaxy/webapps/galaxy/controllers/user.py b/lib/galaxy/webapps/galaxy/controllers/user.py
index 8acb135afb18..46746797d93e 100644
--- a/lib/galaxy/webapps/galaxy/controllers/user.py
+++ b/lib/galaxy/webapps/galaxy/controllers/user.py
@@ -686,7 +686,7 @@ def create( self, trans, cntrller='user', redirect_url='', refresh_frames=[], **
         subscribe_checked = CheckboxField.is_checked( subscribe )
         referer = trans.request.referer or ''
         redirect = kwd.get( 'redirect', referer ).strip()
-        is_admin = cntrller == 'admin' and trans.user_is_admin
+        is_admin = cntrller == 'admin' and trans.user_is_admin()
         if not trans.app.config.allow_user_creation and not trans.user_is_admin():
             message = 'User registration is disabled.  Please contact your local Galaxy administrator for an account.'
             if trans.app.config.error_email_to is not None:
diff --git a/lib/tool_shed/managers/groups.py b/lib/tool_shed/managers/groups.py
index a85fde869590..81b069e9c79e 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 ):

From 0e698813a96f1ad61d797255686f69cf5e6b1280 Mon Sep 17 00:00:00 2001
From: Nate Coraor <nate@bx.psu.edu>
Date: Thu, 19 Oct 2017 09:29:00 -0400
Subject: [PATCH 2/9] [GX-2017-0003]: Fix for the reported issue, only allow
 http, https, and ftp schemes in the data_source tool.

---
 tools/data_source/data_source.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/data_source/data_source.py b/tools/data_source/data_source.py
index 3660c5999f9d..95c65b81bea6 100644
--- a/tools/data_source/data_source.py
+++ b/tools/data_source/data_source.py
@@ -6,7 +6,7 @@
 import sys
 from json import loads, dumps
 
-from six.moves.urllib.parse import urlencode
+from six.moves.urllib.parse import urlencode, urlparse
 from six.moves.urllib.request import urlopen
 
 from galaxy.jobs import TOOL_PROVIDED_JOB_METADATA_FILE
@@ -80,7 +80,7 @@ def __main__():
     for data_dict in job_params[ 'output_data' ]:
         cur_filename = data_dict.get( 'file_name', filename )
         cur_URL = params.get( '%s|%s|URL' % ( GALAXY_PARAM_PREFIX, data_dict[ 'out_data_name' ] ), URL )
-        if not cur_URL:
+        if not cur_URL or urlparse(cur_URL).scheme not in ('http', 'https', 'ftp'):
             open( cur_filename, 'w' ).write( "" )
             stop_err( 'The remote data source application has not sent back a URL parameter in the request.' )
 

From ed045cd570cf6b2198fb496852458194c8e28d6f Mon Sep 17 00:00:00 2001
From: Nate Coraor <nate@bx.psu.edu>
Date: Tue, 12 Sep 2017 15:06:30 -0400
Subject: [PATCH 3/9] 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 ++
 lib/galaxy/webapps/galaxy/api/lda_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/galaxy/webapps/galaxy/controllers/user.py |   2 +-
 lib/tool_shed/managers/groups.py              |   2 +-
 15 files changed, 304 insertions(+), 58 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 2e4a325377f1..181b5ee605b8 100644
--- a/config/galaxy.ini.sample
+++ b/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
diff --git a/lib/galaxy/config.py b/lib/galaxy/config.py
index 49f49bd3c363..85acd5ba7473 100644
--- a/lib/galaxy/config.py
+++ b/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 ) )
diff --git a/lib/galaxy/managers/folders.py b/lib/galaxy/managers/folders.py
index cbb4ad3e9b75..b835ae76e50d 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_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 ) )
diff --git a/lib/galaxy/managers/libraries.py b/lib/galaxy/managers/libraries.py
index 9bcb4d82fd67..337cb3ac8074 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 4a5fc1b00236..0c33ee641854 100644
--- a/lib/galaxy/tools/parameters/grouping.py
+++ b/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 )
diff --git a/lib/galaxy/util/__init__.py b/lib/galaxy/util/__init__.py
index 02055f4963d5..df32a3d29237 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 ):
@@ -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):
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/lda_datasets.py b/lib/galaxy/webapps/galaxy/api/lda_datasets.py
index 14dc75158b79..b75743015778 100644
--- a/lib/galaxy/webapps/galaxy/api/lda_datasets.py
+++ b/lib/galaxy/webapps/galaxy/api/lda_datasets.py
@@ -19,6 +19,7 @@
 from galaxy.exceptions import ObjectNotFound
 from galaxy.managers import folders, roles
 from galaxy.tools.actions import upload_common
+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
@@ -422,29 +423,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
@@ -463,12 +480,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 28d42cd45c02..904c186e2755 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 3df63fa6584a..245410bd6e54 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 87252bd143b6..a05b9c976475 100644
--- a/lib/galaxy/webapps/galaxy/controllers/library_common.py
+++ b/lib/galaxy/webapps/galaxy/controllers/library_common.py
@@ -22,6 +22,7 @@
 from galaxy.security import Action
 from galaxy.tools.actions import upload_common
 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
@@ -71,6 +72,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'
@@ -94,6 +96,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' )
@@ -166,6 +169,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' )
@@ -225,6 +229,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' )
@@ -272,6 +277,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' )
@@ -347,6 +353,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' )
@@ -403,6 +410,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' )
@@ -453,6 +461,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' )
@@ -604,6 +613,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' )
@@ -653,6 +663,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' )
@@ -791,6 +802,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' )
@@ -850,6 +862,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.
@@ -1065,17 +1080,26 @@ def upload_dataset( self, trans, cntrller, library_id, folder_id, replace_datase
         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:
@@ -1268,6 +1292,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' )
@@ -1510,6 +1535,7 @@ def _build_upload_option_select_list( self, trans, upload_option, is_admin, do_n
         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 ) )
@@ -1549,6 +1575,7 @@ def download_dataset_from_folder( self, trans, cntrller, id, library_id=None, **
                                                           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' )
@@ -1597,6 +1624,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' )
@@ -1645,6 +1673,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' )
@@ -1687,6 +1716,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
@@ -1979,6 +2009,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 )
@@ -2109,6 +2140,7 @@ def import_datasets_to_histories( self, trans, cntrller, library_id='', folder_i
                                     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 ) )
@@ -2154,6 +2186,7 @@ def manage_template_inheritance( self, trans, cntrller, item_type, library_id, f
                                                           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 )
@@ -2371,6 +2404,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
@@ -2439,6 +2473,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/galaxy/webapps/galaxy/controllers/user.py b/lib/galaxy/webapps/galaxy/controllers/user.py
index 712bc49be0d5..b3aa7341592e 100644
--- a/lib/galaxy/webapps/galaxy/controllers/user.py
+++ b/lib/galaxy/webapps/galaxy/controllers/user.py
@@ -687,7 +687,7 @@ def create( self, trans, cntrller='user', redirect_url='', refresh_frames=[], **
         subscribe_checked = CheckboxField.is_checked( subscribe )
         referer = trans.request.referer or ''
         redirect = kwd.get( 'redirect', referer ).strip()
-        is_admin = cntrller == 'admin' and trans.user_is_admin
+        is_admin = cntrller == 'admin' and trans.user_is_admin()
         if not trans.app.config.allow_user_creation and not trans.user_is_admin():
             message = 'User registration is disabled.  Please contact your local Galaxy administrator for an account.'
             if trans.app.config.error_email_to is not None:
diff --git a/lib/tool_shed/managers/groups.py b/lib/tool_shed/managers/groups.py
index a85fde869590..81b069e9c79e 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 ):

From 9e672f94588b8eeecee745665162fc92f0158e27 Mon Sep 17 00:00:00 2001
From: Nate Coraor <nate@bx.psu.edu>
Date: Thu, 19 Oct 2017 09:29:00 -0400
Subject: [PATCH 4/9] [GX-2017-0003]: Fix for the reported issue, only allow
 http, https, and ftp schemes in the data_source tool.

---
 tools/data_source/data_source.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/data_source/data_source.py b/tools/data_source/data_source.py
index 53121f1376c1..f513cec5495f 100644
--- a/tools/data_source/data_source.py
+++ b/tools/data_source/data_source.py
@@ -6,7 +6,7 @@
 import sys
 from json import dumps, loads
 
-from six.moves.urllib.parse import urlencode
+from six.moves.urllib.parse import urlencode, urlparse
 from six.moves.urllib.request import urlopen
 
 from galaxy.datatypes import sniff
@@ -80,7 +80,7 @@ def __main__():
     for data_dict in job_params[ 'output_data' ]:
         cur_filename = data_dict.get( 'file_name', filename )
         cur_URL = params.get( '%s|%s|URL' % ( GALAXY_PARAM_PREFIX, data_dict[ 'out_data_name' ] ), URL )
-        if not cur_URL:
+        if not cur_URL or urlparse(cur_URL).scheme not in ('http', 'https', 'ftp'):
             open( cur_filename, 'w' ).write( "" )
             stop_err( 'The remote data source application has not sent back a URL parameter in the request.' )
 

From 1bccbefbffc02803e93913b145f4214b8eb78d54 Mon Sep 17 00:00:00 2001
From: Nate Coraor <nate@bx.psu.edu>
Date: Tue, 12 Sep 2017 15:06:30 -0400
Subject: [PATCH 5/9] 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 ++
 lib/galaxy/webapps/galaxy/api/lda_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/galaxy/webapps/galaxy/controllers/user.py |   2 +-
 lib/tool_shed/managers/groups.py              |   2 +-
 15 files changed, 304 insertions(+), 58 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 be3366072fee..3fbc11ecc1f6 100644
--- a/config/galaxy.ini.sample
+++ b/config/galaxy.ini.sample
@@ -864,6 +864,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
diff --git a/lib/galaxy/config.py b/lib/galaxy/config.py
index d3af12b4305e..32cff60157bb 100644
--- a/lib/galaxy/config.py
+++ b/lib/galaxy/config.py
@@ -370,6 +370,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 ) )
diff --git a/lib/galaxy/managers/folders.py b/lib/galaxy/managers/folders.py
index cbb4ad3e9b75..b835ae76e50d 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_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 ) )
diff --git a/lib/galaxy/managers/libraries.py b/lib/galaxy/managers/libraries.py
index 9bcb4d82fd67..337cb3ac8074 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 f83d66490500..b14b440f8fba 100644
--- a/lib/galaxy/tools/parameters/grouping.py
+++ b/lib/galaxy/tools/parameters/grouping.py
@@ -360,6 +360,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:
@@ -423,6 +424,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 02055f4963d5..df32a3d29237 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 ):
@@ -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):
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/lda_datasets.py b/lib/galaxy/webapps/galaxy/api/lda_datasets.py
index 14dc75158b79..b75743015778 100644
--- a/lib/galaxy/webapps/galaxy/api/lda_datasets.py
+++ b/lib/galaxy/webapps/galaxy/api/lda_datasets.py
@@ -19,6 +19,7 @@
 from galaxy.exceptions import ObjectNotFound
 from galaxy.managers import folders, roles
 from galaxy.tools.actions import upload_common
+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
@@ -422,29 +423,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
@@ -463,12 +480,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 28d42cd45c02..904c186e2755 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 3df63fa6584a..245410bd6e54 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 87252bd143b6..a05b9c976475 100644
--- a/lib/galaxy/webapps/galaxy/controllers/library_common.py
+++ b/lib/galaxy/webapps/galaxy/controllers/library_common.py
@@ -22,6 +22,7 @@
 from galaxy.security import Action
 from galaxy.tools.actions import upload_common
 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
@@ -71,6 +72,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'
@@ -94,6 +96,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' )
@@ -166,6 +169,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' )
@@ -225,6 +229,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' )
@@ -272,6 +277,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' )
@@ -347,6 +353,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' )
@@ -403,6 +410,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' )
@@ -453,6 +461,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' )
@@ -604,6 +613,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' )
@@ -653,6 +663,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' )
@@ -791,6 +802,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' )
@@ -850,6 +862,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.
@@ -1065,17 +1080,26 @@ def upload_dataset( self, trans, cntrller, library_id, folder_id, replace_datase
         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:
@@ -1268,6 +1292,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' )
@@ -1510,6 +1535,7 @@ def _build_upload_option_select_list( self, trans, upload_option, is_admin, do_n
         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 ) )
@@ -1549,6 +1575,7 @@ def download_dataset_from_folder( self, trans, cntrller, id, library_id=None, **
                                                           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' )
@@ -1597,6 +1624,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' )
@@ -1645,6 +1673,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' )
@@ -1687,6 +1716,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
@@ -1979,6 +2009,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 )
@@ -2109,6 +2140,7 @@ def import_datasets_to_histories( self, trans, cntrller, library_id='', folder_i
                                     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 ) )
@@ -2154,6 +2186,7 @@ def manage_template_inheritance( self, trans, cntrller, item_type, library_id, f
                                                           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 )
@@ -2371,6 +2404,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
@@ -2439,6 +2473,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/galaxy/webapps/galaxy/controllers/user.py b/lib/galaxy/webapps/galaxy/controllers/user.py
index 471b20dee7c9..a530bead0f3d 100644
--- a/lib/galaxy/webapps/galaxy/controllers/user.py
+++ b/lib/galaxy/webapps/galaxy/controllers/user.py
@@ -673,7 +673,7 @@ def create( self, trans, cntrller='user', redirect_url='', refresh_frames=[], **
         subscribe_checked = CheckboxField.is_checked( subscribe )
         referer = trans.request.referer or ''
         redirect = kwd.get( 'redirect', referer ).strip()
-        is_admin = cntrller == 'admin' and trans.user_is_admin
+        is_admin = cntrller == 'admin' and trans.user_is_admin()
         if not trans.app.config.allow_user_creation and not trans.user_is_admin():
             message = 'User registration is disabled.  Please contact your local Galaxy administrator for an account.'
             if trans.app.config.error_email_to is not None:
diff --git a/lib/tool_shed/managers/groups.py b/lib/tool_shed/managers/groups.py
index a85fde869590..81b069e9c79e 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 ):

From 658f126e7a387c11df9b5f0a01e0b75e4d84410b Mon Sep 17 00:00:00 2001
From: Nate Coraor <nate@bx.psu.edu>
Date: Thu, 19 Oct 2017 09:29:00 -0400
Subject: [PATCH 6/9] [GX-2017-0003]: Fix for the reported issue, only allow
 http, https, and ftp schemes in the data_source tool.

---
 tools/data_source/data_source.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/data_source/data_source.py b/tools/data_source/data_source.py
index 53121f1376c1..f513cec5495f 100644
--- a/tools/data_source/data_source.py
+++ b/tools/data_source/data_source.py
@@ -6,7 +6,7 @@
 import sys
 from json import dumps, loads
 
-from six.moves.urllib.parse import urlencode
+from six.moves.urllib.parse import urlencode, urlparse
 from six.moves.urllib.request import urlopen
 
 from galaxy.datatypes import sniff
@@ -80,7 +80,7 @@ def __main__():
     for data_dict in job_params[ 'output_data' ]:
         cur_filename = data_dict.get( 'file_name', filename )
         cur_URL = params.get( '%s|%s|URL' % ( GALAXY_PARAM_PREFIX, data_dict[ 'out_data_name' ] ), URL )
-        if not cur_URL:
+        if not cur_URL or urlparse(cur_URL).scheme not in ('http', 'https', 'ftp'):
             open( cur_filename, 'w' ).write( "" )
             stop_err( 'The remote data source application has not sent back a URL parameter in the request.' )
 

From e8a72252effa5fdd888abd0e3462dde3b5c70f6e Mon Sep 17 00:00:00 2001
From: Nate Coraor <nate@bx.psu.edu>
Date: Tue, 12 Sep 2017 15:06:30 -0400
Subject: [PATCH 7/9] 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 ++
 lib/galaxy/webapps/galaxy/api/lda_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/galaxy/webapps/galaxy/controllers/user.py |   2 +-
 lib/tool_shed/managers/groups.py              |   2 +-
 15 files changed, 304 insertions(+), 58 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 3c0ea4283e11..b54b94457fcb 100644
--- a/config/galaxy.ini.sample
+++ b/config/galaxy.ini.sample
@@ -907,6 +907,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
diff --git a/lib/galaxy/config.py b/lib/galaxy/config.py
index ab91f0d64af1..94d5e456a389 100644
--- a/lib/galaxy/config.py
+++ b/lib/galaxy/config.py
@@ -377,6 +377,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 ) )
diff --git a/lib/galaxy/managers/folders.py b/lib/galaxy/managers/folders.py
index cbb4ad3e9b75..b835ae76e50d 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_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 ) )
diff --git a/lib/galaxy/managers/libraries.py b/lib/galaxy/managers/libraries.py
index 9bcb4d82fd67..337cb3ac8074 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 f83d66490500..b14b440f8fba 100644
--- a/lib/galaxy/tools/parameters/grouping.py
+++ b/lib/galaxy/tools/parameters/grouping.py
@@ -360,6 +360,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:
@@ -423,6 +424,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 5f8881a8872d..061b32f62fae 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 ):
@@ -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):
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/lda_datasets.py b/lib/galaxy/webapps/galaxy/api/lda_datasets.py
index 384baeac8192..5a2b499467bb 100644
--- a/lib/galaxy/webapps/galaxy/api/lda_datasets.py
+++ b/lib/galaxy/webapps/galaxy/api/lda_datasets.py
@@ -19,6 +19,7 @@
 from galaxy.managers import folders, 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
@@ -422,29 +423,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
@@ -463,12 +480,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 28d42cd45c02..904c186e2755 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 3df63fa6584a..245410bd6e54 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 9416aa37a4f1..5ac56968e395 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_datase
         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:
@@ -1269,6 +1293,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' )
@@ -1511,6 +1536,7 @@ def _build_upload_option_select_list( self, trans, upload_option, is_admin, do_n
         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 ) )
@@ -1550,6 +1576,7 @@ def download_dataset_from_folder( self, trans, cntrller, id, library_id=None, **
                                                           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' )
@@ -1598,6 +1625,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' )
@@ -1646,6 +1674,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' )
@@ -1688,6 +1717,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
@@ -1980,6 +2010,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 )
@@ -2110,6 +2141,7 @@ def import_datasets_to_histories( self, trans, cntrller, library_id='', folder_i
                                     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 ) )
@@ -2155,6 +2187,7 @@ def manage_template_inheritance( self, trans, cntrller, item_type, library_id, f
                                                           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 )
@@ -2372,6 +2405,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
@@ -2440,6 +2474,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/galaxy/webapps/galaxy/controllers/user.py b/lib/galaxy/webapps/galaxy/controllers/user.py
index 9310661c3fa0..d5434b6e0bce 100644
--- a/lib/galaxy/webapps/galaxy/controllers/user.py
+++ b/lib/galaxy/webapps/galaxy/controllers/user.py
@@ -669,7 +669,7 @@ def create( self, trans, cntrller='user', redirect_url='', refresh_frames=[], **
         subscribe_checked = CheckboxField.is_checked( subscribe )
         referer = trans.request.referer or ''
         redirect = kwd.get( 'redirect', referer ).strip()
-        is_admin = cntrller == 'admin' and trans.user_is_admin
+        is_admin = cntrller == 'admin' and trans.user_is_admin()
         if not trans.app.config.allow_user_creation and not trans.user_is_admin():
             message = 'User registration is disabled.  Please contact your local Galaxy administrator for an account.'
             if trans.app.config.error_email_to is not None:
diff --git a/lib/tool_shed/managers/groups.py b/lib/tool_shed/managers/groups.py
index a85fde869590..81b069e9c79e 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 ):

From 9f8d3ee444ad10038add204ed1c1dc11e636dd9d Mon Sep 17 00:00:00 2001
From: Nate Coraor <nate@bx.psu.edu>
Date: Fri, 29 Sep 2017 14:00:46 -0400
Subject: [PATCH 8/9] Security: Switch GIE Popen() calls to run without
 shell=True to fix an ACE vulnerability.

---
 .../web/base/interactive_environments.py      | 70 ++++++++++---------
 1 file changed, 36 insertions(+), 34 deletions(-)

diff --git a/lib/galaxy/web/base/interactive_environments.py b/lib/galaxy/web/base/interactive_environments.py
index ca29bd9e043c..78804aebd20f 100644
--- a/lib/galaxy/web/base/interactive_environments.py
+++ b/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) )

From cded49354e5fba4b93432294a4518c93b51f259f Mon Sep 17 00:00:00 2001
From: Nate Coraor <nate@bx.psu.edu>
Date: Thu, 19 Oct 2017 09:29:00 -0400
Subject: [PATCH 9/9] [GX-2017-0003]: Fix for the reported issue, only allow
 http, https, and ftp schemes in the data_source tool.

---
 tools/data_source/data_source.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/data_source/data_source.py b/tools/data_source/data_source.py
index 53121f1376c1..f513cec5495f 100644
--- a/tools/data_source/data_source.py
+++ b/tools/data_source/data_source.py
@@ -6,7 +6,7 @@
 import sys
 from json import dumps, loads
 
-from six.moves.urllib.parse import urlencode
+from six.moves.urllib.parse import urlencode, urlparse
 from six.moves.urllib.request import urlopen
 
 from galaxy.datatypes import sniff
@@ -80,7 +80,7 @@ def __main__():
     for data_dict in job_params[ 'output_data' ]:
         cur_filename = data_dict.get( 'file_name', filename )
         cur_URL = params.get( '%s|%s|URL' % ( GALAXY_PARAM_PREFIX, data_dict[ 'out_data_name' ] ), URL )
-        if not cur_URL:
+        if not cur_URL or urlparse(cur_URL).scheme not in ('http', 'https', 'ftp'):
             open( cur_filename, 'w' ).write( "" )
             stop_err( 'The remote data source application has not sent back a URL parameter in the request.' )