diff --git a/NEWS b/NEWS index 1833430b..dda94341 100644 --- a/NEWS +++ b/NEWS @@ -1,9 +1,11 @@ -1.8.8.dev0 +1.8.8 * #370: For the PostgreSQL hook, pass the "PGSSLMODE" environment variable through to Borg when the database's configuration omits the "ssl_mode" option. * #818: Allow the "--repository" flag to match across multiple configuration files. * #820: Fix broken repository detection in the "rcreate" action with Borg 1.4. The issue did not occur with other versions of Borg. + * #822: Fix broken escaping logic in the PostgreSQL hook's "pg_dump_command" option. + * SECURITY: Prevent additional shell injection attacks within the PostgreSQL hook. 1.8.7 * #736: Store included configuration files within each backup archive in support of the "config diff --git a/borgmatic/hooks/postgresql.py b/borgmatic/hooks/postgresql.py index d1cb2d5c..fbbbe4fd 100644 --- a/borgmatic/hooks/postgresql.py +++ b/borgmatic/hooks/postgresql.py @@ -73,9 +73,11 @@ def database_names_to_dump(database, extra_environment, log_prefix, dry_run): if dry_run: return () - psql_command = shlex.split(database.get('psql_command') or 'psql') + psql_command = tuple( + shlex.quote(part) for part in shlex.split(database.get('psql_command') or 'psql') + ) list_command = ( - tuple(psql_command) + psql_command + ('--list', '--no-password', '--no-psqlrc', '--csv', '--tuples-only') + (('--host', database['hostname']) if 'hostname' in database else ()) + (('--port', str(database['port'])) if 'port' in database else ()) @@ -127,7 +129,10 @@ def dump_data_sources(databases, config, log_prefix, dry_run): for database_name in dump_database_names: dump_format = database.get('format', None if database_name == 'all' else 'custom') default_dump_command = 'pg_dumpall' if database_name == 'all' else 'pg_dump' - dump_command = database.get('pg_dump_command') or default_dump_command + dump_command = tuple( + shlex.quote(part) + for part in shlex.split(database.get('pg_dump_command') or default_dump_command) + ) dump_filename = dump.make_data_source_dump_filename( dump_path, database_name, database.get('hostname') ) @@ -138,8 +143,8 @@ def dump_data_sources(databases, config, log_prefix, dry_run): continue command = ( - ( - shlex.quote(dump_command), + dump_command + + ( '--no-password', '--clean', '--if-exists', @@ -242,9 +247,11 @@ def restore_data_source_dump( dump_filename = dump.make_data_source_dump_filename( make_dump_path(config), data_source['name'], data_source.get('hostname') ) - psql_command = shlex.split(data_source.get('psql_command') or 'psql') + psql_command = tuple( + shlex.quote(part) for part in shlex.split(data_source.get('psql_command') or 'psql') + ) analyze_command = ( - tuple(psql_command) + psql_command + ('--no-password', '--no-psqlrc', '--quiet') + (('--host', hostname) if hostname else ()) + (('--port', port) if port else ()) @@ -258,9 +265,12 @@ def restore_data_source_dump( + ('--command', 'ANALYZE') ) use_psql_command = all_databases or data_source.get('format') == 'plain' - pg_restore_command = shlex.split(data_source.get('pg_restore_command') or 'pg_restore') + pg_restore_command = tuple( + shlex.quote(part) + for part in shlex.split(data_source.get('pg_restore_command') or 'pg_restore') + ) restore_command = ( - tuple(psql_command if use_psql_command else pg_restore_command) + (psql_command if use_psql_command else pg_restore_command) + ('--no-password',) + (('--no-psqlrc',) if use_psql_command else ('--if-exists', '--exit-on-error', '--clean')) + (('--dbname', data_source['name']) if not all_databases else ()) diff --git a/setup.py b/setup.py index dd98bc5f..1a24f5a2 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,6 @@ from setuptools import find_packages, setup -VERSION = '1.8.8.dev0' +VERSION = '1.8.8' setup( diff --git a/tests/unit/hooks/test_postgresql.py b/tests/unit/hooks/test_postgresql.py index becf7c92..4e8375b0 100644 --- a/tests/unit/hooks/test_postgresql.py +++ b/tests/unit/hooks/test_postgresql.py @@ -172,11 +172,17 @@ def test_database_names_to_dump_with_all_and_format_excludes_particular_database def test_database_names_to_dump_with_all_and_psql_command_uses_custom_command(): - database = {'name': 'all', 'format': 'custom', 'psql_command': 'docker exec mycontainer psql'} + database = { + 'name': 'all', + 'format': 'custom', + 'psql_command': 'docker exec --workdir * mycontainer psql', + } flexmock(module).should_receive('execute_command_and_capture_output').with_args( ( 'docker', 'exec', + '--workdir', + "'*'", # Should get shell escaped to prevent injection attacks. 'mycontainer', 'psql', '--list', @@ -476,7 +482,7 @@ def test_dump_data_sources_runs_pg_dumpall_for_all_databases(): def test_dump_data_sources_runs_non_default_pg_dump(): - databases = [{'name': 'foo', 'pg_dump_command': 'special_pg_dump'}] + databases = [{'name': 'foo', 'pg_dump_command': 'special_pg_dump --compress *'}] process = flexmock() flexmock(module).should_receive('make_extra_environment').and_return({'PGSSLMODE': 'disable'}) flexmock(module).should_receive('make_dump_path').and_return('') @@ -490,6 +496,8 @@ def test_dump_data_sources_runs_non_default_pg_dump(): flexmock(module).should_receive('execute_command').with_args( ( 'special_pg_dump', + '--compress', + "'*'", # Should get shell escaped to prevent injection attacks. '--no-password', '--clean', '--if-exists', @@ -987,8 +995,8 @@ def test_restore_data_source_dump_runs_non_default_pg_restore_and_psql(): hook_config = [ { 'name': 'foo', - 'pg_restore_command': 'docker exec mycontainer pg_restore', - 'psql_command': 'docker exec mycontainer psql', + 'pg_restore_command': 'docker exec --workdir * mycontainer pg_restore', + 'psql_command': 'docker exec --workdir * mycontainer psql', 'schemas': None, } ] @@ -1001,6 +1009,8 @@ def test_restore_data_source_dump_runs_non_default_pg_restore_and_psql(): ( 'docker', 'exec', + '--workdir', + "'*'", # Should get shell escaped to prevent injection attacks. 'mycontainer', 'pg_restore', '--no-password', @@ -1019,6 +1029,8 @@ def test_restore_data_source_dump_runs_non_default_pg_restore_and_psql(): ( 'docker', 'exec', + '--workdir', + "'*'", # Should get shell escaped to prevent injection attacks. 'mycontainer', 'psql', '--no-password',