From 13dbfff11d765a20623768f2013b1d6322fe1a10 Mon Sep 17 00:00:00 2001 From: Murtuza Zabuawala Date: Thu, 20 Dec 2018 10:09:11 +0000 Subject: [PATCH] Ensure identifiers are properly displayed in the plan viewer. Fixes #3821 --- docs/en_US/release_notes_3_7.rst | 3 +- .../xss_checks_panels_and_query_tool_test.py | 77 ++++++++++++++++++- web/pgadmin/misc/static/explain/js/explain.js | 13 +++- .../static/explain/js/explain_statistics.js | 45 +++++++---- 4 files changed, 120 insertions(+), 18 deletions(-) diff --git a/docs/en_US/release_notes_3_7.rst b/docs/en_US/release_notes_3_7.rst index 966d2d120..65975238a 100644 --- a/docs/en_US/release_notes_3_7.rst +++ b/docs/en_US/release_notes_3_7.rst @@ -27,4 +27,5 @@ Bug fixes | `Bug #3757 `_ - Hide Radio buttons that should not be shown on the maintenance dialogue. | `Bug #3797 `_ - Prevent attempts to bulk-drop schema objects. | `Bug #3798 `_ - Ensure the browser toolbar buttons work in languages other than English. -| `Bug #3805 `_ - Allow horizontal sizing of the edit grid text pop-out. \ No newline at end of file +| `Bug #3805 `_ - Allow horizontal sizing of the edit grid text pop-out. +| `Bug #3821 `_ - Ensure identifiers are properly displayed in the plan viewer. \ No newline at end of file diff --git a/web/pgadmin/feature_tests/xss_checks_panels_and_query_tool_test.py b/web/pgadmin/feature_tests/xss_checks_panels_and_query_tool_test.py index 58fc956cb..edf1c9d2c 100644 --- a/web/pgadmin/feature_tests/xss_checks_panels_and_query_tool_test.py +++ b/web/pgadmin/feature_tests/xss_checks_panels_and_query_tool_test.py @@ -9,6 +9,8 @@ from regression.python_test_utils import test_utils from regression.feature_utils.base_feature_test import BaseFeatureTest +from selenium.webdriver import ActionChains +import sys class CheckForXssFeatureTest(BaseFeatureTest): @@ -55,6 +57,10 @@ class CheckForXssFeatureTest(BaseFeatureTest): self._check_xss_in_query_tool() self.page.close_query_tool() + # Explain module + self._check_xss_in_explain_module() + self.page.close_query_tool() + def after(self): self.page.remove_server(self.server) @@ -68,6 +74,10 @@ class CheckForXssFeatureTest(BaseFeatureTest): self.page.select_tree_item(self.test_table_name) def _check_xss_in_browser_tree(self): + print( + "\n\tChecking the Browser tree for the XSS", + file=sys.stderr, end="" + ) # Fetch the inner html & check for escaped characters source_code = self.page.find_by_xpath( "//*[@id='tree']" @@ -80,6 +90,10 @@ class CheckForXssFeatureTest(BaseFeatureTest): ) def _check_xss_in_properties_tab(self): + print( + "\n\tChecking the Properties tab for the XSS", + file=sys.stderr, end="" + ) self.page.click_tab("Properties") source_code = self.page.find_by_xpath( "//span[contains(@class,'uneditable-input')]" @@ -91,6 +105,10 @@ class CheckForXssFeatureTest(BaseFeatureTest): ) def _check_xss_in_sql_tab(self): + print( + "\n\tChecking the SQL tab for the XSS", + file=sys.stderr, end="" + ) self.page.click_tab("SQL") # Fetch the inner html & check for escaped characters source_code = self.page.find_by_xpath( @@ -106,6 +124,10 @@ class CheckForXssFeatureTest(BaseFeatureTest): # Create any constraint with xss name to test this def _check_xss_in_dependents_tab(self): + print( + "\n\tChecking the Dependents tab for the XSS", + file=sys.stderr, end="" + ) self.page.click_tab("Dependents") source_code = self.page.find_by_xpath( @@ -119,10 +141,17 @@ class CheckForXssFeatureTest(BaseFeatureTest): "Dependents tab (BackGrid)" ) - def _check_xss_in_query_tool(self): + def _open_query_tool(self): self.page.driver.find_element_by_link_text("Tools").click() self.page.find_by_partial_link_text("Query Tool").click() self.page.click_tab('Query -') + + def _check_xss_in_query_tool(self): + print( + "\n\tChecking the SlickGrid cell for the XSS", + file=sys.stderr, end="" + ) + self._open_query_tool() self.page.fill_codemirror_area_with( "select ''" ) @@ -144,6 +173,52 @@ class CheckForXssFeatureTest(BaseFeatureTest): "Query tool (SlickGrid)" ) + def _check_xss_in_explain_module(self): + print( + "\n\tChecking the Graphical Explain plan for the XSS ...", + file=sys.stderr, end="" + ) + self._open_query_tool() + self.page.fill_codemirror_area_with( + 'select * from "{0}"'.format(self.test_table_name) + ) + + query_op = self.page.find_by_id("btn-query-dropdown") + query_op.click() + + self.page.find_by_id("btn-explain").click() + self.page.wait_for_query_tool_loading_indicator_to_disappear() + self.page.click_tab('Explain') + + for idx in range(3): + # Re-try logic + try: + ActionChains(self.driver).move_to_element( + self.driver.find_element_by_css_selector( + 'div.pgadmin-explain-container > svg > g > g > image' + ) + ).perform() + break + except Exception as e: + if idx != 2: + continue + else: + print( + "\n\tUnable to locate the explain container to check" + " the image tooltip for XSS", + file=sys.stderr, end="" + ) + raise + + source_code = self.driver.find_element_by_id( + 'toolTip').get_attribute('innerHTML') + + self._check_escaped_characters( + source_code, + "<h1>X", + "Explain tab (Graphical explain plan)" + ) + def _check_escaped_characters(self, source_code, string_to_find, source): # For XSS we need to search against element's html code assert source_code.find(string_to_find) != - \ diff --git a/web/pgadmin/misc/static/explain/js/explain.js b/web/pgadmin/misc/static/explain/js/explain.js index 88c6c8d78..f3f6f7c16 100644 --- a/web/pgadmin/misc/static/explain/js/explain.js +++ b/web/pgadmin/misc/static/explain/js/explain.js @@ -673,12 +673,12 @@ define('pgadmin.misc.explain', [ key !== 'image_text' && key !== 'xpos' && key !== 'ypos' && key !== 'width' && key !== 'height') { - title += key + ': ' + value + '\n'; + title += `${key}: ${value}\n`; } }); title += ''; - // this.title = Snap.parse(title); + image.append(Snap.parse(title)); image.mouseover(() => { @@ -697,7 +697,14 @@ define('pgadmin.misc.explain', [ key !== 'image_text' && key !== 'xpos' && key !== 'ypos' && key !== 'width' && key !== 'height') { - tooltip.append('' + key + '' + value + ''); + key = _.escape(key); + value = _.escape(value); + tooltip.append(` + + ${key} + ${value} + + `); } }); diff --git a/web/pgadmin/misc/static/explain/js/explain_statistics.js b/web/pgadmin/misc/static/explain/js/explain_statistics.js index 6ae455ff3..b39fc8a90 100644 --- a/web/pgadmin/misc/static/explain/js/explain_statistics.js +++ b/web/pgadmin/misc/static/explain/js/explain_statistics.js @@ -18,7 +18,6 @@ let StatisticsModel = Backbone.Model.extend({ } $('.pg-explain-stats-area').on('mouseover', () => { - // Empty the tooltip content if it has any and add new data toolTipContainer.empty(); if (Object.keys(jit_stats).length == 0 && @@ -33,9 +32,14 @@ let StatisticsModel = Backbone.Model.extend({ if (Object.keys(jit_stats).length > 0){ tooltip.append('JIT:'); _.each(jit_stats, function(value, key) { - tooltip.append('  ' - + key + '' - + value + ''); + key = _.escape(key); + value = _.escape(value); + tooltip.append(` + + ${key} + ${value} + + `); }); } @@ -45,20 +49,35 @@ let StatisticsModel = Backbone.Model.extend({ if (triggers instanceof Object) { _.each(triggers, function(value, key) { if (key === 'Trigger Name') { - tooltip.append('  ' - + key + '' - + value + ''); + key = _.escape(key); + value = _.escape(value); + tooltip.append(` + + ${key} + ${value} + + `); } else { - tooltip.append('    ' - + key + '' - + value + ''); + key = _.escape(key); + value = _.escape(value); + tooltip.append(` + + ${key} + ${value} + + `); } }); } else { - tooltip.append('  ' - + key_id + '' - + triggers + ''); + key_id = _.escape(key_id); + triggers = _.escape(triggers); + tooltip.append(` + + ${key_id} + ${triggers} + + `); } }); }