diff --git a/lib/galaxy/util/__init__.py b/lib/galaxy/util/__init__.py index 061b32f62fa..2955454cdec 100644 --- a/lib/galaxy/util/__init__.py +++ b/lib/galaxy/util/__init__.py @@ -609,14 +609,33 @@ def which(file): 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 + e.g. /a/b/c/d.rst and directory is /a/b, the common prefix is /a/b. + This function isn't used exclusively for security checks, but if it is + used for such checks it is assumed that ``directory`` is a "trusted" path - + supplied by Galaxy or by the admin and ``file`` is something generated by + a tool, configuration, external web server, or user supplied input. local_path_module is used by Pulsar to check Windows paths while running on a POSIX-like system. + + >>> base_dir = tempfile.mkdtemp() + >>> safe_dir = os.path.join(base_dir, "user") + >>> os.mkdir(safe_dir) + >>> good_file = os.path.join(safe_dir, "1") + >>> with open(good_file, "w") as f: f.write("hello") + >>> in_directory(good_file, safe_dir) + True + >>> in_directory("/other/file/is/here.txt", safe_dir) + False + >>> unsafe_link = os.path.join(safe_dir, "2") + >>> os.symlink("/other/file/bad.fasta", unsafe_link) + >>> in_directory(unsafe_link, safe_dir) + False """ if local_path_module != os.path: _safe_contains = importlib.import_module('galaxy.util.path.%s' % local_path_module.__name__).safe_contains else: + directory = os.path.realpath(directory) _safe_contains = safe_contains return _safe_contains(directory, file) diff --git a/lib/galaxy/util/path/__init__.py b/lib/galaxy/util/path/__init__.py index 48a8355ad79..3cde6fb0934 100644 --- a/lib/galaxy/util/path/__init__.py +++ b/lib/galaxy/util/path/__init__.py @@ -31,7 +31,7 @@ def safe_contains(prefix, path, whitelist=None): 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 + symbolic link that points outside of ``prefix``. 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 diff --git a/lib/galaxy/util/path/ntpath.py b/lib/galaxy/util/path/ntpath.py index 5363409b3af..3c86944963a 100644 --- a/lib/galaxy/util/path/ntpath.py +++ b/lib/galaxy/util/path/ntpath.py @@ -2,7 +2,7 @@ """ from __future__ import absolute_import -import ntpath +import ntpath # noqa: I100 See https://github.com/PyCQA/flake8-import-order/pull/115 import sys from . import _build_self diff --git a/lib/galaxy/webapps/galaxy/controllers/library_common.py b/lib/galaxy/webapps/galaxy/controllers/library_common.py index 5ac56968e39..ed22ff9e366 100644 --- a/lib/galaxy/webapps/galaxy/controllers/library_common.py +++ b/lib/galaxy/webapps/galaxy/controllers/library_common.py @@ -278,7 +278,6 @@ class LibraryCommon( BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMet 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' ) @@ -803,7 +802,6 @@ class LibraryCommon( BaseUIController, UsesFormDefinitionsMixin, UsesExtendedMet 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' )