Security fixes for object store paths

lib/galaxy/objectstore/__init__.py

@@ -11,7 +11,7 @@
 import threading
 from xml.etree import ElementTree
 
-from galaxy.util import umask_fix_perms, force_symlink
+from galaxy.util import umask_fix_perms, force_symlink, safe_relpath
 from galaxy.exceptions import ObjectInvalid, ObjectNotFound
 from galaxy.util.sleeper import Sleeper
 from galaxy.util.directory_hash import directory_hash_id
@@ -252,7 +252,17 @@ def _construct_path(self, obj, old_style=False, base_dir=None, dir_only=False, e
                          the composed directory structure does not include a hash id
                          (e.g., /files/dataset_10.dat (old) vs. /files/000/dataset_10.dat (new))
         """
-        base = self.extra_dirs.get(base_dir, self.file_path)
+        base = os.path.abspath(self.extra_dirs.get(base_dir, self.file_path))
+        # extra_dir should never be constructed from provided data but just
+        # make sure there are no shenannigans afoot
+        if extra_dir and extra_dir != os.path.normpath(extra_dir):
+            log.warning('extra_dir is not normalized: %s', extra_dir)
+            raise ObjectInvalid("The requested object is invalid")
+        # ensure that any parent directory references in alt_name would not
+        # result in a path not contained in the directory path constructed here
+        if alt_name and not safe_relpath(alt_name):
+            log.warning('alt_name would locate path outside dir: %s', alt_name)
+            raise ObjectInvalid("The requested object is invalid")
         if old_style:
             if extra_dir is not None:
                 path = os.path.join(base, extra_dir)
@@ -619,9 +629,10 @@ def build_object_store_from_config(config, fsmon=False, config_xml=None):
     elif store == 'irods':
         from .rods import IRODSObjectStore
         return IRODSObjectStore(config=config, config_xml=config_xml)
-    elif store == 'pulsar':
-        from .pulsar import PulsarObjectStore
-        return PulsarObjectStore(config=config, config_xml=config_xml)
+    # Disable the Pulsar object store for now until it receives some attention
+    # elif store == 'pulsar':
+    #    from .pulsar import PulsarObjectStore
+    #    return PulsarObjectStore(config=config, config_xml=config_xml)
     else:
         log.error("Unrecognized object store definition: {0}".format(store))
 

lib/galaxy/objectstore/rods.py

@@ -12,7 +12,8 @@
 from posixpath import basename as path_basename
 from posixpath import dirname as path_dirname
 
-from galaxy.exceptions import ObjectNotFound
+from galaxy.exceptions import ObjectNotFound, ObjectInvalid
+from galaxy.util import safe_relpath
 from ..objectstore import DiskObjectStore, ObjectStore, local_extra_dirs
 
 try:
@@ -71,6 +72,20 @@ def __init__( self, config, file_path=None, extra_dirs=None ):
         log.info( "iRODS data for this instance will be stored in collection: %s, resource: %s", self.root_collection_path, self.default_resource )
 
     def __get_rods_path( self, obj, base_dir=None, dir_only=False, extra_dir=None, extra_dir_at_root=False, alt_name=None, strip_dat=True, **kwargs ):
+        # extra_dir should never be constructed from provided data but just
+        # make sure there are no shenannigans afoot
+        if extra_dir and extra_dir != os.path.normpath(extra_dir):
+            log.warning('extra_dir is not normalized: %s', extra_dir)
+            raise ObjectInvalid("The requested object is invalid")
+        # ensure that any parent directory references in alt_name would not
+        # result in a path not contained in the directory path constructed here
+        if alt_name:
+            if not safe_relpath(alt_name):
+                log.warning('alt_name would locate path outside dir: %s', alt_name)
+                raise ObjectInvalid("The requested object is invalid")
+            # alt_name can contain parent directory references, but iRODS will
+            # not follow them, so if they are valid we normalize them out
+            alt_name = os.path.normpath(alt_name)
         path = ""
         if extra_dir is not None:
             path = extra_dir

lib/galaxy/objectstore/s3.py

@@ -12,8 +12,8 @@
 
 from datetime import datetime
 
-from galaxy.exceptions import ObjectNotFound
-from galaxy.util import string_as_bool, umask_fix_perms
+from galaxy.exceptions import ObjectNotFound, ObjectInvalid
+from galaxy.util import string_as_bool, umask_fix_perms, safe_relpath
 from galaxy.util.directory_hash import directory_hash_id
 from galaxy.util.sleeper import Sleeper
 from .s3_multipart_upload import multipart_upload
@@ -208,6 +208,20 @@ def _fix_permissions(self, rel_path):
                 umask_fix_perms( path, self.config.umask, 0666, self.config.gid )
 
     def _construct_path(self, obj, base_dir=None, dir_only=None, extra_dir=None, extra_dir_at_root=False, alt_name=None, obj_dir=False, **kwargs):
+        # extra_dir should never be constructed from provided data but just
+        # make sure there are no shenannigans afoot
+        if extra_dir and extra_dir != os.path.normpath(extra_dir):
+            log.warning('extra_dir is not normalized: %s', extra_dir)
+            raise ObjectInvalid("The requested object is invalid")
+        # ensure that any parent directory references in alt_name would not
+        # result in a path not contained in the directory path constructed here
+        if alt_name:
+            if not safe_relpath(alt_name):
+                log.warning('alt_name would locate path outside dir: %s', alt_name)
+                raise ObjectInvalid("The requested object is invalid")
+            # alt_name can contain parent directory references, but S3 will not
+            # follow them, so if they are valid we normalize them out
+            alt_name = os.path.normpath(alt_name)
         rel_path = os.path.join(*directory_hash_id(obj.id))
         if extra_dir is not None:
             if extra_dir_at_root: