Merge pull request #4856 from jmchilton/release_16.07_fix_in_directory

[16.07] Fix galaxy.util.in_directory()...

lib/galaxy/util/__init__.py

@@ -605,14 +605,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)
 

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