summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSimon Charette <charette.s@gmail.com>2020-10-04 18:31:04 -0400
committerMariusz Felisiak <felisiak.mariusz@gmail.com>2020-10-30 10:12:52 +0100
commit384ac0990ff414526ec47381845dae79b8e3ddfe (patch)
treeef21e47574574a7c79908cbfdcc72bfc2723d71a
parenteb25fdb620f9fc3e2bcde916f9db2c06f53a3cd7 (diff)
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.
-rw-r--r--django/db/backends/mysql/client.py12
-rw-r--r--tests/dbshell/test_mysql.py42
2 files changed, 40 insertions, 14 deletions
diff --git a/django/db/backends/mysql/client.py b/django/db/backends/mysql/client.py
index 79032c1207..95442a32b0 100644
--- a/django/db/backends/mysql/client.py
+++ b/django/db/backends/mysql/client.py
@@ -7,6 +7,7 @@ class DatabaseClient(BaseDatabaseClient):
@classmethod
def settings_to_cmd_args_env(cls, settings_dict, parameters):
args = [cls.executable_name]
+ env = None
db = settings_dict['OPTIONS'].get('db', settings_dict['NAME'])
user = settings_dict['OPTIONS'].get('user', settings_dict['USER'])
password = settings_dict['OPTIONS'].get(
@@ -27,7 +28,14 @@ class DatabaseClient(BaseDatabaseClient):
if user:
args += ["--user=%s" % user]
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 '/' in host:
args += ["--socket=%s" % host]
@@ -46,4 +54,4 @@ class DatabaseClient(BaseDatabaseClient):
if db:
args += [db]
args.extend(parameters)
- return args, None
+ return args, env
diff --git a/tests/dbshell/test_mysql.py b/tests/dbshell/test_mysql.py
index 374c466059..b002a5d274 100644
--- a/tests/dbshell/test_mysql.py
+++ b/tests/dbshell/test_mysql.py
@@ -1,3 +1,7 @@
+import subprocess
+import sys
+from pathlib import Path
+
from django.db.backends.mysql.client import DatabaseClient
from django.test import SimpleTestCase
@@ -16,12 +20,11 @@ class MySqlDbshellCommandTestCase(SimpleTestCase):
expected_args = [
'mysql',
'--user=someuser',
- '--password=somepassword',
'--host=somehost',
'--port=444',
'somedbname',
]
- expected_env = None
+ expected_env = {'MYSQL_PWD': 'somepassword'}
self.assertEqual(
self.settings_to_cmd_args_env({
'NAME': 'somedbname',
@@ -41,12 +44,11 @@ class MySqlDbshellCommandTestCase(SimpleTestCase):
expected_args = [
'mysql',
'--user=optionuser',
- '--password=optionpassword',
'--host=optionhost',
'--port=%s' % options_port,
'optiondbname',
]
- expected_env = None
+ expected_env = {'MYSQL_PWD': 'optionpassword'}
self.assertEqual(
self.settings_to_cmd_args_env({
'NAME': 'settingdbname',
@@ -69,12 +71,11 @@ class MySqlDbshellCommandTestCase(SimpleTestCase):
expected_args = [
'mysql',
'--user=someuser',
- '--password=optionpassword',
'--host=somehost',
'--port=444',
'somedbname',
]
- expected_env = None
+ expected_env = {'MYSQL_PWD': 'optionpassword'}
self.assertEqual(
self.settings_to_cmd_args_env({
'NAME': 'somedbname',
@@ -91,13 +92,12 @@ class MySqlDbshellCommandTestCase(SimpleTestCase):
expected_args = [
'mysql',
'--user=someuser',
- '--password=somepassword',
'--host=somehost',
'--port=444',
'--default-character-set=utf8',
'somedbname',
]
- expected_env = None
+ expected_env = {'MYSQL_PWD': 'somepassword'}
self.assertEqual(
self.settings_to_cmd_args_env({
'NAME': 'somedbname',
@@ -114,11 +114,10 @@ class MySqlDbshellCommandTestCase(SimpleTestCase):
expected_args = [
'mysql',
'--user=someuser',
- '--password=somepassword',
'--socket=/path/to/mysql.socket.file',
'somedbname',
]
- expected_env = None
+ expected_env = {'MYSQL_PWD': 'somepassword'}
self.assertEqual(
self.settings_to_cmd_args_env({
'NAME': 'somedbname',
@@ -135,7 +134,6 @@ class MySqlDbshellCommandTestCase(SimpleTestCase):
expected_args = [
'mysql',
'--user=someuser',
- '--password=somepassword',
'--host=somehost',
'--port=444',
'--ssl-ca=sslca',
@@ -143,7 +141,7 @@ class MySqlDbshellCommandTestCase(SimpleTestCase):
'--ssl-key=sslkey',
'somedbname',
]
- expected_env = None
+ expected_env = {'MYSQL_PWD': 'somepassword'}
self.assertEqual(
self.settings_to_cmd_args_env({
'NAME': 'somedbname',
@@ -177,3 +175,23 @@ class MySqlDbshellCommandTestCase(SimpleTestCase):
),
(['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))