From 02ecc82e73f12e84e54c3947ada943d80f746c87 Mon Sep 17 00:00:00 2001 From: Aditya Toshniwal Date: Mon, 18 Sep 2023 14:01:11 +0530 Subject: [PATCH] Fix a security related issue where an authenticated user can run remote command using validate binary path API (CVE-2023-5002). #6763 (#6764) --- docs/en_US/release_notes_7_7.rst | 1 + web/pgadmin/misc/__init__.py | 4 +++- web/pgadmin/misc/file_manager/__init__.py | 20 ++++++++++---------- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/docs/en_US/release_notes_7_7.rst b/docs/en_US/release_notes_7_7.rst index 80a3e3213..f47e7647b 100644 --- a/docs/en_US/release_notes_7_7.rst +++ b/docs/en_US/release_notes_7_7.rst @@ -41,3 +41,4 @@ Bug fixes | `Issue #6712 `_ - Ensure that Materialized view size fields in "Statistics" should be human-readable. | `Issue #6730 `_ - Fix an issue where changing the password shows success but the new password is not working. | `Issue #6738 `_ - Fix an issue where login form doesn't appear if internal auth source is removed. + | `Issue #6764 `_ - Fix a security related issue where an authenticated user can run remote command using validate binary path API (CVE-2023-5002). diff --git a/web/pgadmin/misc/__init__.py b/web/pgadmin/misc/__init__.py index 87061006c..15adccf51 100644 --- a/web/pgadmin/misc/__init__.py +++ b/web/pgadmin/misc/__init__.py @@ -268,9 +268,11 @@ def validate_binary_path(): if not os.path.exists(binary_path): current_app.logger.warning('Invalid binary path.') raise Exception() + # escape double quotes to avoid command injection. # Get the output of the '--version' command version_string = \ - subprocess.getoutput('"{0}" --version'.format(full_path)) + subprocess.getoutput(r'"{0}" --version'.format( + full_path.replace('"', '""'))) # Get the version number by splitting the result string version_string.split(") ", 1)[1].split('.', 1)[0] except Exception: diff --git a/web/pgadmin/misc/file_manager/__init__.py b/web/pgadmin/misc/file_manager/__init__.py index cf076213e..5a8d7064d 100644 --- a/web/pgadmin/misc/file_manager/__init__.py +++ b/web/pgadmin/misc/file_manager/__init__.py @@ -829,9 +829,9 @@ class Filemanager(): try: os.rename(oldpath_sys, newpath_sys) - except Exception as e: + except OSError as e: return internal_server_error("{0} {1}".format( - gettext('There was an error renaming the file:'), e)) + gettext('There was an error renaming the file:'), e.strerror)) return { 'Old Path': old, @@ -859,9 +859,9 @@ class Filemanager(): os.rmdir(orig_path) else: os.remove(orig_path) - except Exception as e: + except OSError as e: return internal_server_error("{0} {1}".format( - gettext('There was an error deleting the file:'), e)) + gettext('There was an error deleting the file:'), e.strerror)) return make_json_response(status=200) @@ -903,9 +903,9 @@ class Filemanager(): if not data: break f.write(data) - except Exception as e: + except OSError as e: return internal_server_error("{0} {1}".format( - gettext('There was an error adding the file:'), e)) + gettext('There was an error adding the file:'), e.strerror)) Filemanager.check_access_permission(the_dir, path) @@ -1021,10 +1021,10 @@ class Filemanager(): if ex.strerror == 'Permission denied': return unauthorized(str(ex.strerror)) else: - return internal_server_error(str(ex)) + return internal_server_error(str(ex.strerror)) except Exception as ex: - return internal_server_error(str(ex)) + return internal_server_error(str(ex.strerror)) # Remove root storage path from error message # when running in Server mode @@ -1054,8 +1054,8 @@ class Filemanager(): self.get_new_name(user_dir, path, name) try: os.mkdir(create_path) - except Exception as e: - return internal_server_error(str(e)) + except OSError as e: + return internal_server_error(str(e.strerror)) result = { 'Parent': path,