Ensure strings are properly encoded in the Query History. Fixes #4349

docs/en_US/release_notes_4_9.rst

@@ -26,5 +26,5 @@ Bug fixes
 | `Bug #4320 <https://redmine.postgresql.org/issues/4320>`_ - Fix issue where SSH tunnel connection using password is failing, it's regression of Master Password.
 | `Bug #4329 <https://redmine.postgresql.org/issues/4329>`_ - Fix an initialisation error when two functions with parameters are debugged in parallel.
 | `Bug #4343 <https://redmine.postgresql.org/issues/4343>`_ - Fix issue where property dialog of column should open properly for EPAS v12.
-| `Bug #4350 <https://redmine.postgresql.org/issues/4350>`_ - Ensure we include the CSRF token when uploading files.
-| `Bug #4357 <https://redmine.postgresql.org/issues/4357>`_ - Fix connection restoration issue when pgAdmin server is restarted and the page is refreshed.
+| `Bug #4349 <https://redmine.postgresql.org/issues/4349>`_ - Ensure strings are properly encoded in the Query History.
+| `Bug #4350 <https://redmine.postgresql.org/issues/4350>`_ - Ensure we include the CSRF token when uploading files.| `Bug #4357 <https://redmine.postgresql.org/issues/4357>`_ - Fix connection restoration issue when pgAdmin server is restarted and the page is refreshed.

web/pgadmin/feature_tests/xss_checks_panels_and_query_tool_test.py

@@ -63,6 +63,7 @@ class CheckForXssFeatureTest(BaseFeatureTest):
         # Query tool
         self.page.open_query_tool()
         self._check_xss_in_query_tool()
+        self._check_xss_in_query_tool_history()
         self.page.close_query_tool()
 
         # Explain module
@@ -177,6 +178,61 @@ class CheckForXssFeatureTest(BaseFeatureTest):
             "Query tool (SlickGrid)"
         )
 
+    def _check_xss_in_query_tool_history(self):
+        print(
+            "\n\tChecking the query tool history for the XSS",
+            file=sys.stderr, end=""
+        )
+
+        self.page.fill_codemirror_area_with(
+            "select '<script>alert(1)</script>"
+        )
+        self.page.find_by_id("btn-flash").click()
+
+        self.page.click_tab('Query History')
+
+        # Check for history entry
+        history_ele = self.page.find_by_css_selector(
+            ".query-history div.query-group:first-child"
+            " .list-item.selected .query"
+        )
+
+        source_code = history_ele.get_attribute('innerHTML')
+
+        self._check_escaped_characters(
+            source_code,
+            '&lt;script&gt;alert(1)&lt;/script&gt;',
+            "Query tool (History Entry)"
+        )
+
+        # Check for history details message
+        history_ele = self.page.find_by_css_selector(
+            ".query-detail .content-value"
+        )
+
+        source_code = history_ele.get_attribute('innerHTML')
+
+        self._check_escaped_characters(
+            source_code,
+            '&lt;script&gt;alert(1)&lt;/script&gt;',
+            "Query tool (History Details-Message)"
+        )
+
+        # Check for history details error message
+        history_ele = self.page.find_by_css_selector(
+            ".query-detail .history-error-text"
+        )
+
+        source_code = history_ele.get_attribute('innerHTML')
+
+        self._check_escaped_characters(
+            source_code,
+            '&lt;script&gt;alert(1)&lt;/script&gt;',
+            "Query tool (History Details-Error)"
+        )
+
+        self.page.click_tab('Query Editor')
+
     def _check_xss_in_explain_module(self):
         print(
             "\n\tChecking the Graphical Explain plan for the XSS ...",

web/pgadmin/static/js/sqleditor/history/query_history_details.js

@@ -117,7 +117,7 @@ export default class QueryHistoryDetails {
   updateMessageContent() {
     this.$message_content
       .empty()
-      .append(`<pre class='content-value'>${this.entry.message}</pre>`);
+      .append(`<pre class='content-value'>${_.escape(this.entry.message)}</pre>`);
   }
 
   updateErrorMessage() {
@@ -125,10 +125,8 @@ export default class QueryHistoryDetails {
       this.$errMsgBlock.removeClass('d-none');
       this.$errMsgBlock.empty().append(
         `<div class='history-error-text'>
-                    <span>Error Message</span> ${this.parseErrorMessage(
-    this.entry.message
-  )}
-                </div>`
+            <span>Error Message</span>${_.escape(this.parseErrorMessage(this.entry.message))}
+        </div>`
       );
     } else {
       this.$errMsgBlock.addClass('d-none');

web/pgadmin/static/js/sqleditor/history/query_history_entries.js

@@ -1,5 +1,6 @@
 import moment from 'moment';
 import $ from 'jquery';
+import _ from 'underscore';
 
 const ARROWUP = 38;
 const ARROWDOWN = 40;
@@ -68,7 +69,7 @@ export class QueryHistoryItem {
     this.$el = $(
       `<li class='list-item' tabindex='0' data-key='${this.dataKey()}'>
           <div class='entry ${this.entry.status ? '' : 'error'}'>
-              <div class='query'>${this.entry.query}</div>
+              <div class='query'>${_.escape(this.entry.query)}</div>
               <div class='other-info'>
               <div class='timestamp'>${this.formatDate(this.entry.start_time)}</div>
               </div>

web/regression/javascript/history/query_history_spec.js

@@ -260,7 +260,7 @@ describe('QueryHistory', () => {
           });
 
           it('displays the error message on top of the details pane', () => {
-            expect(queryDetail.text()).toContain('Error Message message from second sql query');
+            expect(queryDetail.text()).toContain('Error Messagemessage from second sql query');
           });
         });
       });
@@ -322,7 +322,7 @@ describe('QueryHistory', () => {
         });
 
         it('displays fourth query SQL in the right pane', () => {
-          expect(queryDetail.text()).toContain('Error Message unexpected error from fourth sql message');
+          expect(queryDetail.text()).toContain('Error Messageunexpected error from fourth sql message');
         });
       });
     });