pgadmin-org
/
pgadmin4
mirror of https://github.com/pgadmin-org/pgadmin4.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Ensure object names in external process dialogues are properly escaped. Fixes #3872

pull/20/head
Murtuza Zabuawala 2019-01-24 16:34:18 +00:00 committed by Dave Page
parent 42c7ae372f
commit d9fc9fdc4d
9 changed files with 121 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
docs/en_US/release_notes_4_2.rst
View File

@ -34,6 +34,7 @@ Bug fixes
| `Bug #3858 <https://redmine.postgresql.org/issues/3858>`_ - Drop-down should be closed when click on any other toolbar button.
| `Bug #3862 <https://redmine.postgresql.org/issues/3862>`_ - Fixed keyboard navigation for dialog tabs.
| `Bug #3871 <https://redmine.postgresql.org/issues/3871>`_ - Fixed alignment of tree arrow icons for Internet Explorer.
| `Bug #3872 <https://redmine.postgresql.org/issues/3872>`_ - Ensure object names in external process dialogues are properly escaped.
| `Bug #3891 <https://redmine.postgresql.org/issues/3891>`_ - Correct order of Save and Cancel button for json/jsonb editing.
| `Bug #3897 <https://redmine.postgresql.org/issues/3897>`_ - Data should be updated properly for FTS Configurations, FTS Dictionaries, FTS Parsers and FTS Templates.
| `Bug #3908 <https://redmine.postgresql.org/issues/3908>`_ - Fixed keyboard navigation for Select2 and Privilege cell in Backgrid.

9
web/pgadmin/browser/static/js/error.js
View File

@ -14,7 +14,10 @@ function(gettext, _, alertify, pgAdmin) {
_.extend(pgAdmin.Browser, {
report_error: function(title, message, info) {
var text = '<div class="panel-group" id="accordion" role="tablist" aria-multiselectable="true">\
title = _.escape(title);
message = _.escape(message);
info = _.escape(info);
let text = '<div class="panel-group" id="accordion" role="tablist" aria-multiselectable="true">\
<div class="panel panel-default">\
<div class="panel-heading" role="tab" id="headingOne">\
<h4 class="panel-title">\
@ -23,7 +26,7 @@ function(gettext, _, alertify, pgAdmin) {
</h4>\
</div>\
<div id="collapseOne" class="panel-collapse collapse show" role="tabpanel" aria-labelledby="headingOne">\
<div class="panel-body" style="overflow: auto;">' + unescape(message) + '</div>\
<div class="panel-body" style="overflow: auto;">' + message + '</div>\
</div>\
</div>';
@ -36,7 +39,7 @@ function(gettext, _, alertify, pgAdmin) {
</h4>\
</div>\
<div id="collapseTwo" class="panel-collapse collapse" role="tabpanel" aria-labelledby="headingTwo">\
<div class="panel-body" style="overflow: auto;">' + unescape(info) + '</div>\
<div class="panel-body" style="overflow: auto;">' + info + '</div>\
</div>\
</div>\
</div>';

79
web/pgadmin/feature_tests/pg_utilities_backup_restore_test.py
View File

@ -20,7 +20,14 @@ class PGUtilitiesBackupFeatureTest(BaseFeatureTest):
""" This class test PG utilities - Backup and Restore test scenarios """
scenarios = [
("Test for PG utilities - Backup and Restore", dict())
("Test for PG utilities - Backup and Restore", dict(
database_name="pg_utility_test_db",
is_xss_check=False,
)),
("Test for XSS in Backup and Restore", dict(
database_name="<h1>test_me</h1>",
is_xss_check=True,
)),
]
def before(self):
@ -39,9 +46,9 @@ class PGUtilitiesBackupFeatureTest(BaseFeatureTest):
self.server['port'],
self.server['sslmode']
)
test_utils.drop_database(connection, "pg_utility_test_db")
test_utils.drop_database(connection, self.database_name)
test_utils.create_database(self.server, "pg_utility_test_db")
test_utils.create_database(self.server, self.database_name)
self.page.add_server(self.server)
self.wait = WebDriverWait(self.page.driver, 20)
@ -49,7 +56,7 @@ class PGUtilitiesBackupFeatureTest(BaseFeatureTest):
def runTest(self):
self.page.toggle_open_server(self.server['name'])
self.page.toggle_open_tree_item('Databases')
self.page.toggle_open_tree_item('pg_utility_test_db')
self.page.toggle_open_tree_item(self.database_name)
# Backup
self.driver.find_element_by_link_text("Tools").click()
@ -75,22 +82,28 @@ class PGUtilitiesBackupFeatureTest(BaseFeatureTest):
self.page.find_by_css_selector(
".pg-bg-more-details").click()
command = self.page.find_by_css_selector(
".bg-process-details .bg-detailed-desc").text
self.assertIn(self.server['name'], str(command))
self.assertIn("from database 'pg_utility_test_db'", str(command))
# Check for XSS in Backup details
if self.is_xss_check:
self._check_detailed_window_for_xss('Backup')
else:
command = self.page.find_by_css_selector(
".bg-process-details .bg-detailed-desc").text
# On windows a modified path may be shown so skip this test
if os.name is not 'nt':
self.assertIn("test_backup", str(command))
self.assertIn(self.server['name'], str(command))
self.assertIn("from database 'pg_utility_test_db'", str(command))
self.assertIn("pg_dump", str(command))
# On windows a modified path may be shown so skip this test
if os.name is not 'nt':
self.assertIn("test_backup", str(command))
self.assertIn("pg_dump", str(command))
backup_file = None
if command:
backup_file = command[int(command.find('--file')) +
8:int(command.find('--host')) - 2]
backup_file = None
if command:
backup_file = command[int(command.find('--file')) +
8:int(command.find('--host')) - 2]
self.page.find_by_xpath("//div[contains(@class,'wcFloatingFocus')"
"]//div[contains(@class,'fa-close')]").click()
@ -117,14 +130,19 @@ class PGUtilitiesBackupFeatureTest(BaseFeatureTest):
self.page.find_by_css_selector(
".pg-bg-more-details").click()
command = self.page.find_by_css_selector(
".bg-process-details .bg-detailed-desc").text
self.assertIn(self.server['name'], str(command))
if os.name is not 'nt':
self.assertIn("test_backup", str(command))
# Check for XSS in Restore details
if self.is_xss_check:
self._check_detailed_window_for_xss('Restore')
else:
command = self.page.find_by_css_selector(
".bg-process-details .bg-detailed-desc").text
self.assertIn("pg_restore", str(command))
self.assertIn(self.server['name'], str(command))
if os.name is not 'nt':
self.assertIn("test_backup", str(command))
self.assertIn("pg_restore", str(command))
self.page.find_by_xpath("//div[contains(@class,'wcFloatingFocus')]"
"//div[contains(@class,'fa-close')]").click()
@ -144,4 +162,19 @@ class PGUtilitiesBackupFeatureTest(BaseFeatureTest):
self.server['port'],
self.server['sslmode']
)
test_utils.drop_database(connection, "pg_utility_test_db")
test_utils.drop_database(connection, self.database_name)
def _check_detailed_window_for_xss(self, tool_name):
source_code = self.page.find_by_css_selector(
".bg-process-details .bg-detailed-desc"
).get_attribute('innerHTML')
self._check_escaped_characters(
source_code,
'&lt;h1&gt;test_me&lt;/h1&gt;',
'{0} detailed window'.format(tool_name)
)
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) != - \
1, "{0} might be vulnerable to XSS ".format(source)

27
web/pgadmin/feature_tests/pg_utilities_maintenance_test.py
View File

@ -22,12 +22,20 @@ class PGUtilitiesMaintenanceFeatureTest(BaseFeatureTest):
("Test for PG maintenance: database", dict(
database_name='pg_maintenance',
table_name='pg_maintenance_table',
test_level='database'
test_level='database',
is_xss_check=False,
)),
("Test for PG maintenance: table", dict(
database_name='pg_maintenance',
table_name='pg_maintenance_table',
test_level='table'
test_level='table',
is_xss_check=False,
)),
("Test for XSS in maintenance dialog", dict(
database_name='pg_maintenance',
table_name='<h1>test_me</h1>',
test_level='table',
is_xss_check=True,
)),
]
@ -85,6 +93,16 @@ class PGUtilitiesMaintenanceFeatureTest(BaseFeatureTest):
self.assertEquals(command, "VACUUM "
"(VERBOSE)\nRunning Query:"
"\nVACUUM VERBOSE;")
elif self.is_xss_check and self.test_level == 'table':
# Check for XSS in the dialog
source_code = self.page.find_by_css_selector(
".bg-process-details .bg-detailed-desc"
).get_attribute('innerHTML')
self._check_escaped_characters(
source_code,
'&lt;h1&gt;test_me&lt;/h1&gt;',
'Maintenance detailed window'
)
else:
self.assertEquals(command, "VACUUM "
"(VERBOSE)\nRunning Query:"
@ -106,3 +124,8 @@ class PGUtilitiesMaintenanceFeatureTest(BaseFeatureTest):
self.server['sslmode']
)
test_utils.drop_database(connection, self.database_name)
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) != - \
1, "{0} might be vulnerable to XSS ".format(source)

9
web/pgadmin/misc/bgprocess/static/js/bgprocess.js
View File

@ -292,13 +292,13 @@ define('misc.bgprocess', [
let content = $(`
<div class="card">
<div class="card-header bg-primary d-flex">
<div>${_.escape(self.type_desc)}</div>
<div>${self.type_desc}</div>
<div class="ml-auto">
<button class="btn btn-sm-sq btn-primary pg-bg-close"><i class="fa fa-lg fa-close"></i></button>
</div>
</div>
<div class="card-body px-2">
<div class="py-1">${_.unescape(self.desc)}</div>
<div class="py-1">${self.desc}</div>
<div class="py-1">${self.stime.toString()}</div>
<div class="d-flex py-1">
<div class="my-auto mr-2">
@ -388,8 +388,7 @@ define('misc.bgprocess', [
is_new = true;
panel = this.panel =
pgBrowser.BackgroundProcessObsorver.create_panel();
panel.title('Process Watcher - ' + _.escape(self.type_desc));
panel.title('Process Watcher - ' + self.type_desc);
panel.focus();
}
@ -416,7 +415,7 @@ define('misc.bgprocess', [
self.logs[0].scrollTop = self.logs[0].scrollHeight;
});
// set bgprocess detailed description
$header.find('.bg-detailed-desc').html(_.unescape(self.detailed_desc));
$header.find('.bg-detailed-desc').html(self.detailed_desc);
}
// set bgprocess start time

30
web/pgadmin/tools/backup/__init__.py
View File

@ -140,6 +140,9 @@ class BackupMessage(IProcessDesc):
@property
def message(self):
name, host, port = self.get_server_details()
name = html.safe_str(name)
host = html.safe_str(host)
port = html.safe_str(port)
if self.backup_type == BACKUP.OBJECT:
return _(
@ -149,7 +152,7 @@ class BackupMessage(IProcessDesc):
"{0} ({1}:{2})".format(
name, host, port
),
self.database
html.safe_str(self.database)
)
if self.backup_type == BACKUP.GLOBALS:
return _("Backing up the global objects on "
@ -174,34 +177,31 @@ class BackupMessage(IProcessDesc):
res = '<div>'
if self.backup_type == BACKUP.OBJECT:
res += _(
msg = _(
"Backing up an object on the server '{0}' "
"from database '{1}'..."
).format(
"{0} ({1}:{2})".format(
html.safe_str(name),
html.safe_str(host),
html.safe_str(port),
name, host, port
),
html.safe_str(self.database)
self.database
)
res += html.safe_str(msg)
elif self.backup_type == BACKUP.GLOBALS:
res += _("Backing up the global objects on "
"the server '{0}'...").format(
msg = _("Backing up the global objects on "
"the server '{0}'...").format(
"{0} ({1}:{2})".format(
html.safe_str(name),
html.safe_str(host),
html.safe_str(port)
name, host, port
)
)
res += html.safe_str(msg)
elif self.backup_type == BACKUP.SERVER:
res += _("Backing up the server '{0}'...").format(
msg = _("Backing up the server '{0}'...").format(
"{0} ({1}:{2})".format(
html.safe_str(name),
html.safe_str(host),
html.safe_str(port)
name, host, port
)
)
res += html.safe_str(msg)
else:
# It should never reach here.
res += "Backup"

7
web/pgadmin/tools/import_export/__init__.py
View File

@ -83,7 +83,6 @@ class IEMessage(IProcessDesc):
def cmdArg(x):
if x:
x = html.safe_str(x)
x = x.replace('\\', '\\\\')
x = x.replace('"', '\\"')
x = x.replace('""', '\\"')
@ -116,7 +115,11 @@ class IEMessage(IProcessDesc):
"Copying table data '{0}.{1}' on database '{2}' "
"and server ({3}:{4})"
).format(
self.schema, self.table, self.database, s.host, s.port
html.safe_str(self.schema),
html.safe_str(self.table),
html.safe_str(self.database),
html.safe_str(s.host),
html.safe_str(s.port)
)
@property

7
web/pgadmin/tools/restore/__init__.py
View File

@ -73,7 +73,6 @@ class RestoreMessage(IProcessDesc):
def cmdArg(x):
if x:
x = html.safe_str(x)
x = x.replace('\\', '\\\\')
x = x.replace('"', '\\"')
x = x.replace('""', '\\"')
@ -106,7 +105,11 @@ class RestoreMessage(IProcessDesc):
name, host, port = self.get_server_details()
return _("Restoring backup on the server '{0}'").format(
"{0} ({1}:{2})".format(name, host, port),
"{0} ({1}:{2})".format(
html.safe_str(name),
html.safe_str(host),
html.safe_str(port)
),
)
@property

8
web/pgadmin/utils/driver/psycopg2/connection.py
View File

@ -623,10 +623,10 @@ WHERE
# "unicode_escape" will convert single backslash to double
# backslash, so we will have to replace/revert them again
# to store the correct value into the database.
# if isinstance(val, six.string_types):
# modified_val = val.encode('unicode_escape')\
# .decode('raw_unicode_escape')\
# .replace("\\\\", "\\")
if isinstance(val, six.string_types):
modified_val = val.encode('unicode_escape')\
.decode('raw_unicode_escape')\
.replace("\\\\", "\\")
params[key] = modified_val