From 1f51b4abebacb502f0691537d2954e757c5df018 Mon Sep 17 00:00:00 2001
From: Carl Eberhard <carlfeberhard@gmail.com>
Date: Tue, 25 Nov 2014 14:22:08 -0500
Subject: [PATCH] Sanitize tag output in tagging_common and user.tags_used;
 protect against closing tags in bootstrapped JSON
 (http://benalpert.com/2012/08/03/preventing-xss-json.html); minor fixes

---
 lib/galaxy/tags/tag_handler.py                | 28 ++++++++++++++--
 lib/galaxy/util/json.py                       | 10 +++---
 lib/galaxy/util/validation.py                 |  4 +--
 lib/galaxy/web/framework/helpers/__init__.py  |  1 -
 lib/galaxy/webapps/galaxy/controllers/tag.py  |  4 +--
 templates/galaxy_client_app.mako              | 12 +++++--
 templates/tagging_common.mako                 | 10 +++---
 templates/webapps/galaxy/galaxy.masthead.mako | 32 ++-----------------
 8 files changed, 53 insertions(+), 48 deletions(-)

diff --git a/lib/galaxy/tags/tag_handler.py b/lib/galaxy/tags/tag_handler.py
index 02ae6aa0e63..0883c921a7b 100644
--- a/lib/galaxy/tags/tag_handler.py
+++ b/lib/galaxy/tags/tag_handler.py
@@ -1,5 +1,7 @@
-import re, logging
-from sqlalchemy.sql.expression import func, and_
+import re
+import logging
+from sqlalchemy.sql.expression import func
+from sqlalchemy.sql.expression import and_
 from sqlalchemy.sql import select
 
 log = logging.getLogger( __name__ )
@@ -25,12 +27,15 @@ class TagHandler( object ):
         self.key_value_separators = "=:"
         # Initialize with known classes - add to this in subclasses.
         self.item_tag_assoc_info = {}
+
     def get_tag_assoc_class( self, item_class ):
         """Returns tag association class for item class."""
         return self.item_tag_assoc_info[item_class.__name__].tag_assoc_class
+
     def get_id_col_in_item_tag_assoc_table( self, item_class ):
         """Returns item id column in class' item-tag association table."""
         return self.item_tag_assoc_info[item_class.__name__].item_id_col
+
     def get_community_tags( self, trans, item=None, limit=None ):
         """Returns community tags for an item."""
         # Get item-tag association class.
@@ -58,6 +63,7 @@ class TagHandler( object ):
             tag_id = row[0]
             community_tags.append( self.get_tag_by_id( trans, tag_id ) )
         return community_tags
+
     def get_tool_tags( self, trans ):
         result_set = trans.sa_session.execute( select( columns=[ trans.app.model.ToolTagAssociation.table.c.tag_id ],
                                                        from_obj=trans.app.model.ToolTagAssociation.table ).distinct() )
@@ -67,6 +73,7 @@ class TagHandler( object ):
             tag_id = row[0]
             tags.append( self.get_tag_by_id( trans, tag_id ) )
         return tags
+
     def remove_item_tag( self, trans, user, item, tag_name ):
         """Remove a tag from an item."""
         # Get item tag association.
@@ -78,6 +85,7 @@ class TagHandler( object ):
             item.tags.remove( item_tag_assoc )
             return True
         return False
+
     def delete_item_tags( self, trans, user, item ):
         """Delete tags from an item."""
         # Delete item-tag associations.
@@ -85,6 +93,7 @@ class TagHandler( object ):
             trans.sa_session.delete( tag )
         # Delete tags from item.
         del item.tags[:]
+
     def item_has_tag( self, trans, user, item, tag ):
         """Returns true if item is has a given tag."""
         # Get tag name.
@@ -97,6 +106,7 @@ class TagHandler( object ):
         if item_tag_assoc:
             return True
         return False
+
     def apply_item_tag( self, trans, user, item, name, value=None ):
         # Use lowercase name for searching/creating tag.
         lc_name = name.lower()
@@ -124,6 +134,7 @@ class TagHandler( object ):
         item_tag_assoc.user_value = value
         item_tag_assoc.value = lc_value
         return item_tag_assoc
+
     def apply_item_tags( self, trans, user, item, tags_str ):
         """Apply tags to an item."""
         # Parse tags.
@@ -131,6 +142,7 @@ class TagHandler( object ):
         # Apply each tag.
         for name, value in parsed_tags.items():
             self.apply_item_tag( trans, user, item, name, value )
+
     def get_tags_str( self, tags ):
         """Build a string from an item's tags."""
         # Return empty string if there are no tags.
@@ -144,14 +156,17 @@ class TagHandler( object ):
                 tag_str += ":" + tag.user_value
             tags_str_list.append( tag_str )
         return ", ".join( tags_str_list )
+
     def get_tag_by_id( self, trans, tag_id ):
         """Get a Tag object from a tag id."""
         return trans.sa_session.query( trans.app.model.Tag ).filter_by( id=tag_id ).first()
+
     def get_tag_by_name( self, trans, tag_name ):
         """Get a Tag object from a tag name (string)."""
         if tag_name:
             return trans.sa_session.query( trans.app.model.Tag ).filter_by( name=tag_name.lower() ).first()
         return None
+
     def _create_tag( self, trans, tag_str ):
         """Create a Tag object from a tag string."""
         tag_hierarchy = tag_str.split( self.hierarchy_separator )
@@ -169,6 +184,7 @@ class TagHandler( object ):
             parent_tag = tag
             tag_prefix = tag.name + self.hierarchy_separator
         return tag
+
     def _get_or_create_tag( self, trans, tag_str ):
         """Get or create a Tag object from a tag string."""
         # Scrub tag; if tag is None after being scrubbed, return None.
@@ -181,6 +197,7 @@ class TagHandler( object ):
         if tag is None:
             tag = self._create_tag( trans, scrubbed_tag_str )
         return tag
+
     def _get_item_tag_assoc( self, user, item, tag_name ):
         """
         Return ItemTagAssociation object for a user, item, and tag string; returns None if there is
@@ -191,6 +208,7 @@ class TagHandler( object ):
             if ( item_tag_assoc.user == user ) and ( item_tag_assoc.user_tname == scrubbed_tag_name ):
                 return item_tag_assoc
         return None
+
     def parse_tags( self, tag_str ):
         """
         Returns a list of raw (tag-name, value) pairs derived from a string; method scrubs tag names and values as well.
@@ -210,6 +228,7 @@ class TagHandler( object ):
             scrubbed_value = self._scrub_tag_value( nv_pair[1] )
             name_value_pairs[scrubbed_name] = scrubbed_value
         return name_value_pairs
+
     def _scrub_tag_value( self, value ):
         """Scrub a tag value."""
         # Gracefully handle None:
@@ -219,6 +238,7 @@ class TagHandler( object ):
         reg_exp = re.compile( '\s' )
         scrubbed_value = re.sub( reg_exp, "", value )
         return scrubbed_value
+
     def _scrub_tag_name( self, name ):
         """Scrub a tag name."""
         # Gracefully handle None:
@@ -234,12 +254,14 @@ class TagHandler( object ):
         if len( scrubbed_name ) < self.min_tag_len or len( scrubbed_name ) > self.max_tag_len:
             return None
         return scrubbed_name
+
     def _scrub_tag_name_list( self, tag_name_list ):
         """Scrub a tag name list."""
         scrubbed_tag_list = list()
         for tag in tag_name_list:
             scrubbed_tag_list.append( self._scrub_tag_name( tag ) )
         return scrubbed_tag_list
+
     def _get_name_value_pair( self, tag_str ):
         """Get name, value pair from a tag string."""
         # Use regular expression to parse name, value.
@@ -250,6 +272,7 @@ class TagHandler( object ):
             name_value_pair.append( None )
         return name_value_pair
 
+
 class GalaxyTagHandler( TagHandler ):
     def __init__( self ):
         from galaxy import model
@@ -271,6 +294,7 @@ class GalaxyTagHandler( TagHandler ):
                                                                       model.VisualizationTagAssociation,
                                                                       model.VisualizationTagAssociation.table.c.visualization_id )
 
+
 class CommunityTagHandler( TagHandler ):
     def __init__( self ):
         from galaxy.webapps.tool_shed import model
diff --git a/lib/galaxy/util/json.py b/lib/galaxy/util/json.py
index 923a09f9724..1cc3262e719 100644
--- a/lib/galaxy/util/json.py
+++ b/lib/galaxy/util/json.py
@@ -57,7 +57,7 @@ def swap_inf_nan( val ):
         return val
 
 
-def safe_dumps(*args, **kwargs):
+def safe_dumps( *args, **kwargs ):
     """
     This is a wrapper around dumps that encodes Infinity and NaN values.  It's a
     fairly rare case (which will be low in request volume).  Basically, we tell
@@ -65,10 +65,12 @@ def safe_dumps(*args, **kwargs):
     re-encoding.
     """
     try:
-        dumped = json.dumps(*args, allow_nan=False, **kwargs)
+        dumped = json.dumps( *args, allow_nan=False, **kwargs )
     except ValueError:
-        obj = swap_inf_nan(copy.deepcopy(args[0]))
-        dumped = json.dumps(obj, allow_nan=False, **kwargs)
+        obj = swap_inf_nan( copy.deepcopy( args[0] ) )
+        dumped = json.dumps( obj, allow_nan=False, **kwargs )
+    if kwargs.get( 'escape_closing_tags', True ):
+        return dumped.replace( '</', '<\\/' )
     return dumped
 
 
diff --git a/lib/galaxy/util/validation.py b/lib/galaxy/util/validation.py
index d1cbfd806ee..40d20c4ef60 100644
--- a/lib/galaxy/util/validation.py
+++ b/lib/galaxy/util/validation.py
@@ -8,8 +8,8 @@ from galaxy.util.sanitize_html import sanitize_html
 
 def validate_and_sanitize_basestring( key, val ):
     if not isinstance( val, basestring ):
-            raise exceptions.RequestParameterInvalidException( '%s must be a string or unicode: %s'
-                                                               % ( key, str( type( val ) ) ) )
+        raise exceptions.RequestParameterInvalidException( '%s must be a string or unicode: %s'
+                                                           % ( key, str( type( val ) ) ) )
     return unicode( sanitize_html( val, 'utf-8', 'text/html' ), 'utf-8' )
 
 
diff --git a/lib/galaxy/web/framework/helpers/__init__.py b/lib/galaxy/web/framework/helpers/__init__.py
index 3ab9d78b316..f692f8e5622 100644
--- a/lib/galaxy/web/framework/helpers/__init__.py
+++ b/lib/galaxy/web/framework/helpers/__init__.py
@@ -111,4 +111,3 @@ def is_true ( val ):
     Returns true if input is a boolean and true or is a string and looks like a true value.
     """
     return val == True or val in [ 'True', 'true', 'T', 't' ]
-
diff --git a/lib/galaxy/webapps/galaxy/controllers/tag.py b/lib/galaxy/webapps/galaxy/controllers/tag.py
index acc83500f25..bbdf366bbf5 100644
--- a/lib/galaxy/webapps/galaxy/controllers/tag.py
+++ b/lib/galaxy/webapps/galaxy/controllers/tag.py
@@ -64,7 +64,7 @@ class TagsController ( BaseUIController, UsesTagsMixin ):
         trans.log_action( user, unicode( "untag" ), context, params )
 
     # Retag an item. All previous tags are deleted and new tags are applied.
-    #@web.expose
+    @web.expose
     @web.require_login( "Apply a new set of tags to an item; previous tags are deleted." )
     def retag_async( self, trans, item_id=None, item_class=None, new_tags=None ):
         """
@@ -73,7 +73,7 @@ class TagsController ( BaseUIController, UsesTagsMixin ):
         # Apply tags.
         item = self._get_item( trans, item_class, trans.security.decode_id( item_id ) )
         user = trans.user
-        self.get_tag_handler( trans ).delete_item_tags( trans, item )
+        self.get_tag_handler( trans ).delete_item_tags( trans, user, item )
         self.get_tag_handler( trans ).apply_item_tags( trans, user, item, new_tags.encode( 'utf-8' ) )
         trans.sa_session.flush()
 
diff --git a/templates/galaxy_client_app.mako b/templates/galaxy_client_app.mako
index b160071c34e..0dfeb520a24 100644
--- a/templates/galaxy_client_app.mako
+++ b/templates/galaxy_client_app.mako
@@ -15,7 +15,7 @@
         //TODO: global...
         %for key in kwargs:
             ( window.bootstrapped = window.bootstrapped || {} )[ '${key}' ] = (
-                ${ h.dumps( kwargs[ key ], indent=( 2 if trans.debug else 0 ) )} );
+                ${ h.dumps( kwargs[ key ], indent=( 2 if trans.debug else 0 ) ).replace( '</', '<\\/' ) } );
         %endfor
         define( 'bootstrapped-data', function(){
             return window.bootstrapped;
@@ -76,11 +76,17 @@ ${ h.dumps( get_config_dict() )}
                 user_dict = trans.user.to_dict( view='element',
                     value_mapper={ 'id': trans.security.encode_id, 'total_disk_usage': float } )
                 user_dict[ 'quota_percent' ] = trans.app.quota_agent.get_percent( trans=trans )
+                user_dict[ 'is_admin' ] = trans.user_is_admin()
 
                 # tags used
                 users_api_controller = trans.webapp.api_controllers[ 'users' ]
-                user_dict[ 'tags_used' ] = users_api_controller.get_user_tags_used( trans, user=trans.user )
-                user_dict[ 'is_admin' ] = trans.user_is_admin()
+                tags_used = []
+                for tag in users_api_controller.get_user_tags_used( trans, user=trans.user ):
+                    tag = tag | h
+                    if tag:
+                        tags_used.append( tag )
+                user_dict[ 'tags_used' ] = tags_used
+
                 return user_dict
 
             usage = 0
diff --git a/templates/tagging_common.mako b/templates/tagging_common.mako
index 02e28e86a02..35ccfb4550a 100644
--- a/templates/tagging_common.mako
+++ b/templates/tagging_common.mako
@@ -19,7 +19,7 @@
 ## Render HTML for a list of tags.
 <%def name="render_tagging_element_html(elt_id=None, tags=None, editable=True, use_toggle_link=True, input_size='15', in_form=False, tag_type='individual', render_add_tag_button=True)">
     ## Useful attributes.
-    <% 
+    <%
         num_tags = len( tags )
     %>
     <div class="tag-element"
@@ -50,6 +50,7 @@
                     elif isinstance( tag, ItemTagAssociation ):
                         tag_name = tag.user_tname
                         tag_value = tag.user_value
+                        
                     ## Convert tag name, value to unicode.
                     if isinstance( tag_name, str ):
                         tag_name = unicode( escape( tag_name ), 'utf-8' )
@@ -61,7 +62,7 @@
                         tag_str = tag_name
                 %>
                 <span class="tag-button">
-                    <span class="tag-name">${tag_str}</span>
+                    <span class="tag-name">${tag_str | h}</span>
                     %if editable:
                         <img class="delete-tag-img" src="${h.url_for('/static/images/delete_tag_icon_gray.png')}"/>
                     %endif
@@ -186,10 +187,11 @@
             ## Build dict of tag name, values.
             tag_names_and_values = dict()
             for tag in item_tags:
-                tag_name = tag.user_tname
+                tag_name = escape( tag.user_tname )
                 tag_value = ""
                 if tag.value is not None:
-                    tag_value = tag.user_value
+                    tag_value = escape( tag.user_value )
+
                 ## Tag names and values may be string or unicode object.
                 if isinstance( tag_name, str ):
                     tag_names_and_values[unicode(tag_name, 'utf-8')] = unicode(tag_value, 'utf-8')
diff --git a/templates/webapps/galaxy/galaxy.masthead.mako b/templates/webapps/galaxy/galaxy.masthead.mako
index ae0e2899180..223cb093626 100644
--- a/templates/webapps/galaxy/galaxy.masthead.mako
+++ b/templates/webapps/galaxy/galaxy.masthead.mako
@@ -1,32 +1,4 @@
-## get user data
-<%def name="get_user_json()">
-<%
-    """Bootstrapping user API JSON"""
-    #TODO: move into common location (poss. BaseController)
-    if trans.user:
-        user_dict = trans.user.to_dict( view='element', value_mapper={ 'id': trans.security.encode_id,
-                                                                             'total_disk_usage': float } )
-        user_dict[ 'quota_percent' ] = trans.app.quota_agent.get_percent( trans=trans )
-        users_api_controller = trans.webapp.api_controllers[ 'users' ]
-        user_dict[ 'tags_used' ] = users_api_controller.get_user_tags_used( trans, user=trans.user )
-        user_dict[ 'is_admin' ] = trans.user_is_admin()
-    else:
-        usage = 0
-        percent = None
-        try:
-            usage = trans.app.quota_agent.get_usage( trans, history=trans.history )
-            percent = trans.app.quota_agent.get_percent( trans=trans, usage=usage )
-        except AssertionError, assertion:
-            # no history for quota_agent.get_usage assertion
-            pass
-        user_dict = {
-            'total_disk_usage'      : int( usage ),
-            'nice_total_disk_usage' : util.nice_size( usage ),
-            'quota_percent'         : percent
-        }
-    return user_dict
-%>
-</%def>
+<%namespace file="/galaxy_client_app.mako" import="get_user_json" />
 
 ## masthead head generator
 <%def name="load(active_view = None)">
@@ -87,7 +59,7 @@
         ], function( mod_masthead, mod_menu, mod_modal, mod_frame, GalaxyUpload, user, quotameter ){
             if( !Galaxy.currUser ){
                 // this doesn't need to wait for the page being readied
-                Galaxy.currUser = new user.User(${ h.dumps( get_user_json(), indent=2 ) });
+                Galaxy.currUser = new user.User(${ h.dumps( masthead_config[ 'user' ][ 'json' ], indent=2 ) });
             }
 
             $(function() {