Skip to content
Permalink
Browse files

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

  • Loading branch information...
natefoo committed Sep 12, 2017
1 parent 8e199a3 commit 93a8bfc7cb5e9c3395c5057910ec39d68ad787b4
@@ -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
@@ -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 ) )
@@ -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 ) )
@@ -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 )
@@ -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 )
@@ -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):
@@ -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',
)
@@ -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)
@@ -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)

0 comments on commit 93a8bfc

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