Refs #32061 -- Prevented password leak on MySQL dbshell crash.

The usage of the --password flag when invoking the mysql CLI has the
potential of exposing the password in plain text if the command happens
to crash due to the inclusion of args provided to
subprocess.run(check=True) in the string representation of the
subprocess.CalledProcessError exception raised on non-zero return code.

Since this has the potential of leaking the password to logging
facilities configured to capture crashes (e.g. sys.excepthook, Sentry)
it's safer to rely on the MYSQL_PWD environment variable instead even
if its usage is discouraged due to potential leak through the ps
command on old flavors of Unix.

Thanks Charlie Denton for reporting the issue to the security team.

Refs #24999.
This commit is contained in:
Simon Charette 2020-10-04 18:31:04 -04:00 committed by Mariusz Felisiak
parent eb25fdb620
commit 384ac0990f
2 changed files with 40 additions and 14 deletions

View File

@ -7,6 +7,7 @@ class DatabaseClient(BaseDatabaseClient):
@classmethod @classmethod
def settings_to_cmd_args_env(cls, settings_dict, parameters): def settings_to_cmd_args_env(cls, settings_dict, parameters):
args = [cls.executable_name] args = [cls.executable_name]
env = None
db = settings_dict['OPTIONS'].get('db', settings_dict['NAME']) db = settings_dict['OPTIONS'].get('db', settings_dict['NAME'])
user = settings_dict['OPTIONS'].get('user', settings_dict['USER']) user = settings_dict['OPTIONS'].get('user', settings_dict['USER'])
password = settings_dict['OPTIONS'].get( password = settings_dict['OPTIONS'].get(
@ -27,7 +28,14 @@ class DatabaseClient(BaseDatabaseClient):
if user: if user:
args += ["--user=%s" % user] args += ["--user=%s" % user]
if password: if password:
args += ["--password=%s" % password] # The MYSQL_PWD environment variable usage is discouraged per
# MySQL's documentation due to the possibility of exposure through
# `ps` on old Unix flavors but --password suffers from the same
# flaw on even more systems. Usage of an environment variable also
# prevents password exposure if the subprocess.run(check=True) call
# raises a CalledProcessError since the string representation of
# the latter includes all of the provided `args`.
env = {'MYSQL_PWD': password}
if host: if host:
if '/' in host: if '/' in host:
args += ["--socket=%s" % host] args += ["--socket=%s" % host]
@ -46,4 +54,4 @@ class DatabaseClient(BaseDatabaseClient):
if db: if db:
args += [db] args += [db]
args.extend(parameters) args.extend(parameters)
return args, None return args, env

View File

@ -1,3 +1,7 @@
import subprocess
import sys
from pathlib import Path
from django.db.backends.mysql.client import DatabaseClient from django.db.backends.mysql.client import DatabaseClient
from django.test import SimpleTestCase from django.test import SimpleTestCase
@ -16,12 +20,11 @@ class MySqlDbshellCommandTestCase(SimpleTestCase):
expected_args = [ expected_args = [
'mysql', 'mysql',
'--user=someuser', '--user=someuser',
'--password=somepassword',
'--host=somehost', '--host=somehost',
'--port=444', '--port=444',
'somedbname', 'somedbname',
] ]
expected_env = None expected_env = {'MYSQL_PWD': 'somepassword'}
self.assertEqual( self.assertEqual(
self.settings_to_cmd_args_env({ self.settings_to_cmd_args_env({
'NAME': 'somedbname', 'NAME': 'somedbname',
@ -41,12 +44,11 @@ class MySqlDbshellCommandTestCase(SimpleTestCase):
expected_args = [ expected_args = [
'mysql', 'mysql',
'--user=optionuser', '--user=optionuser',
'--password=optionpassword',
'--host=optionhost', '--host=optionhost',
'--port=%s' % options_port, '--port=%s' % options_port,
'optiondbname', 'optiondbname',
] ]
expected_env = None expected_env = {'MYSQL_PWD': 'optionpassword'}
self.assertEqual( self.assertEqual(
self.settings_to_cmd_args_env({ self.settings_to_cmd_args_env({
'NAME': 'settingdbname', 'NAME': 'settingdbname',
@ -69,12 +71,11 @@ class MySqlDbshellCommandTestCase(SimpleTestCase):
expected_args = [ expected_args = [
'mysql', 'mysql',
'--user=someuser', '--user=someuser',
'--password=optionpassword',
'--host=somehost', '--host=somehost',
'--port=444', '--port=444',
'somedbname', 'somedbname',
] ]
expected_env = None expected_env = {'MYSQL_PWD': 'optionpassword'}
self.assertEqual( self.assertEqual(
self.settings_to_cmd_args_env({ self.settings_to_cmd_args_env({
'NAME': 'somedbname', 'NAME': 'somedbname',
@ -91,13 +92,12 @@ class MySqlDbshellCommandTestCase(SimpleTestCase):
expected_args = [ expected_args = [
'mysql', 'mysql',
'--user=someuser', '--user=someuser',
'--password=somepassword',
'--host=somehost', '--host=somehost',
'--port=444', '--port=444',
'--default-character-set=utf8', '--default-character-set=utf8',
'somedbname', 'somedbname',
] ]
expected_env = None expected_env = {'MYSQL_PWD': 'somepassword'}
self.assertEqual( self.assertEqual(
self.settings_to_cmd_args_env({ self.settings_to_cmd_args_env({
'NAME': 'somedbname', 'NAME': 'somedbname',
@ -114,11 +114,10 @@ class MySqlDbshellCommandTestCase(SimpleTestCase):
expected_args = [ expected_args = [
'mysql', 'mysql',
'--user=someuser', '--user=someuser',
'--password=somepassword',
'--socket=/path/to/mysql.socket.file', '--socket=/path/to/mysql.socket.file',
'somedbname', 'somedbname',
] ]
expected_env = None expected_env = {'MYSQL_PWD': 'somepassword'}
self.assertEqual( self.assertEqual(
self.settings_to_cmd_args_env({ self.settings_to_cmd_args_env({
'NAME': 'somedbname', 'NAME': 'somedbname',
@ -135,7 +134,6 @@ class MySqlDbshellCommandTestCase(SimpleTestCase):
expected_args = [ expected_args = [
'mysql', 'mysql',
'--user=someuser', '--user=someuser',
'--password=somepassword',
'--host=somehost', '--host=somehost',
'--port=444', '--port=444',
'--ssl-ca=sslca', '--ssl-ca=sslca',
@ -143,7 +141,7 @@ class MySqlDbshellCommandTestCase(SimpleTestCase):
'--ssl-key=sslkey', '--ssl-key=sslkey',
'somedbname', 'somedbname',
] ]
expected_env = None expected_env = {'MYSQL_PWD': 'somepassword'}
self.assertEqual( self.assertEqual(
self.settings_to_cmd_args_env({ self.settings_to_cmd_args_env({
'NAME': 'somedbname', 'NAME': 'somedbname',
@ -177,3 +175,23 @@ class MySqlDbshellCommandTestCase(SimpleTestCase):
), ),
(['mysql', 'somedbname', '--help'], None), (['mysql', 'somedbname', '--help'], None),
) )
def test_crash_password_does_not_leak(self):
# The password doesn't leak in an exception that results from a client
# crash.
args, env = DatabaseClient.settings_to_cmd_args_env(
{
'NAME': 'somedbname',
'USER': 'someuser',
'PASSWORD': 'somepassword',
'HOST': 'somehost',
'PORT': 444,
'OPTIONS': {},
},
[],
)
fake_client = Path(__file__).with_name('fake_client.py')
args[0:1] = [sys.executable, str(fake_client)]
with self.assertRaises(subprocess.CalledProcessError) as ctx:
subprocess.run(args, check=True, env=env)
self.assertNotIn('somepassword', str(ctx.exception))