Fix broken escaping logic for "pg_dump_command" (#822) + bonus shell injection fixes.
This commit is contained in:
parent
75d11aa9cd
commit
6fa5dff79b
4 changed files with 39 additions and 15 deletions
4
NEWS
4
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
|
* #370: For the PostgreSQL hook, pass the "PGSSLMODE" environment variable through to Borg when the
|
||||||
database's configuration omits the "ssl_mode" option.
|
database's configuration omits the "ssl_mode" option.
|
||||||
* #818: Allow the "--repository" flag to match across multiple configuration files.
|
* #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
|
* #820: Fix broken repository detection in the "rcreate" action with Borg 1.4. The issue did not
|
||||||
occur with other versions of Borg.
|
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
|
1.8.7
|
||||||
* #736: Store included configuration files within each backup archive in support of the "config
|
* #736: Store included configuration files within each backup archive in support of the "config
|
||||||
|
|
|
@ -73,9 +73,11 @@ def database_names_to_dump(database, extra_environment, log_prefix, dry_run):
|
||||||
if dry_run:
|
if dry_run:
|
||||||
return ()
|
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 = (
|
list_command = (
|
||||||
tuple(psql_command)
|
psql_command
|
||||||
+ ('--list', '--no-password', '--no-psqlrc', '--csv', '--tuples-only')
|
+ ('--list', '--no-password', '--no-psqlrc', '--csv', '--tuples-only')
|
||||||
+ (('--host', database['hostname']) if 'hostname' in database else ())
|
+ (('--host', database['hostname']) if 'hostname' in database else ())
|
||||||
+ (('--port', str(database['port'])) if 'port' 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:
|
for database_name in dump_database_names:
|
||||||
dump_format = database.get('format', None if database_name == 'all' else 'custom')
|
dump_format = database.get('format', None if database_name == 'all' else 'custom')
|
||||||
default_dump_command = 'pg_dumpall' if database_name == 'all' else 'pg_dump'
|
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_filename = dump.make_data_source_dump_filename(
|
||||||
dump_path, database_name, database.get('hostname')
|
dump_path, database_name, database.get('hostname')
|
||||||
)
|
)
|
||||||
|
@ -138,8 +143,8 @@ def dump_data_sources(databases, config, log_prefix, dry_run):
|
||||||
continue
|
continue
|
||||||
|
|
||||||
command = (
|
command = (
|
||||||
(
|
dump_command
|
||||||
shlex.quote(dump_command),
|
+ (
|
||||||
'--no-password',
|
'--no-password',
|
||||||
'--clean',
|
'--clean',
|
||||||
'--if-exists',
|
'--if-exists',
|
||||||
|
@ -242,9 +247,11 @@ def restore_data_source_dump(
|
||||||
dump_filename = dump.make_data_source_dump_filename(
|
dump_filename = dump.make_data_source_dump_filename(
|
||||||
make_dump_path(config), data_source['name'], data_source.get('hostname')
|
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 = (
|
analyze_command = (
|
||||||
tuple(psql_command)
|
psql_command
|
||||||
+ ('--no-password', '--no-psqlrc', '--quiet')
|
+ ('--no-password', '--no-psqlrc', '--quiet')
|
||||||
+ (('--host', hostname) if hostname else ())
|
+ (('--host', hostname) if hostname else ())
|
||||||
+ (('--port', port) if port else ())
|
+ (('--port', port) if port else ())
|
||||||
|
@ -258,9 +265,12 @@ def restore_data_source_dump(
|
||||||
+ ('--command', 'ANALYZE')
|
+ ('--command', 'ANALYZE')
|
||||||
)
|
)
|
||||||
use_psql_command = all_databases or data_source.get('format') == 'plain'
|
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 = (
|
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-password',)
|
||||||
+ (('--no-psqlrc',) if use_psql_command else ('--if-exists', '--exit-on-error', '--clean'))
|
+ (('--no-psqlrc',) if use_psql_command else ('--if-exists', '--exit-on-error', '--clean'))
|
||||||
+ (('--dbname', data_source['name']) if not all_databases else ())
|
+ (('--dbname', data_source['name']) if not all_databases else ())
|
||||||
|
|
2
setup.py
2
setup.py
|
@ -1,6 +1,6 @@
|
||||||
from setuptools import find_packages, setup
|
from setuptools import find_packages, setup
|
||||||
|
|
||||||
VERSION = '1.8.8.dev0'
|
VERSION = '1.8.8'
|
||||||
|
|
||||||
|
|
||||||
setup(
|
setup(
|
||||||
|
|
|
@ -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():
|
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(
|
flexmock(module).should_receive('execute_command_and_capture_output').with_args(
|
||||||
(
|
(
|
||||||
'docker',
|
'docker',
|
||||||
'exec',
|
'exec',
|
||||||
|
'--workdir',
|
||||||
|
"'*'", # Should get shell escaped to prevent injection attacks.
|
||||||
'mycontainer',
|
'mycontainer',
|
||||||
'psql',
|
'psql',
|
||||||
'--list',
|
'--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():
|
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()
|
process = flexmock()
|
||||||
flexmock(module).should_receive('make_extra_environment').and_return({'PGSSLMODE': 'disable'})
|
flexmock(module).should_receive('make_extra_environment').and_return({'PGSSLMODE': 'disable'})
|
||||||
flexmock(module).should_receive('make_dump_path').and_return('')
|
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(
|
flexmock(module).should_receive('execute_command').with_args(
|
||||||
(
|
(
|
||||||
'special_pg_dump',
|
'special_pg_dump',
|
||||||
|
'--compress',
|
||||||
|
"'*'", # Should get shell escaped to prevent injection attacks.
|
||||||
'--no-password',
|
'--no-password',
|
||||||
'--clean',
|
'--clean',
|
||||||
'--if-exists',
|
'--if-exists',
|
||||||
|
@ -987,8 +995,8 @@ def test_restore_data_source_dump_runs_non_default_pg_restore_and_psql():
|
||||||
hook_config = [
|
hook_config = [
|
||||||
{
|
{
|
||||||
'name': 'foo',
|
'name': 'foo',
|
||||||
'pg_restore_command': 'docker exec mycontainer pg_restore',
|
'pg_restore_command': 'docker exec --workdir * mycontainer pg_restore',
|
||||||
'psql_command': 'docker exec mycontainer psql',
|
'psql_command': 'docker exec --workdir * mycontainer psql',
|
||||||
'schemas': None,
|
'schemas': None,
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
|
@ -1001,6 +1009,8 @@ def test_restore_data_source_dump_runs_non_default_pg_restore_and_psql():
|
||||||
(
|
(
|
||||||
'docker',
|
'docker',
|
||||||
'exec',
|
'exec',
|
||||||
|
'--workdir',
|
||||||
|
"'*'", # Should get shell escaped to prevent injection attacks.
|
||||||
'mycontainer',
|
'mycontainer',
|
||||||
'pg_restore',
|
'pg_restore',
|
||||||
'--no-password',
|
'--no-password',
|
||||||
|
@ -1019,6 +1029,8 @@ def test_restore_data_source_dump_runs_non_default_pg_restore_and_psql():
|
||||||
(
|
(
|
||||||
'docker',
|
'docker',
|
||||||
'exec',
|
'exec',
|
||||||
|
'--workdir',
|
||||||
|
"'*'", # Should get shell escaped to prevent injection attacks.
|
||||||
'mycontainer',
|
'mycontainer',
|
||||||
'psql',
|
'psql',
|
||||||
'--no-password',
|
'--no-password',
|
||||||
|
|
Loading…
Reference in a new issue