Database password visible on debug page in local variable

[bytej4ck]

In debug page view, secrets are not visible due to masked with '*'. When there is mysql db connection error due to unreachable db server: self.connection = self.get_new_connection(conn_params) exposes db password under Local vars dropdown.

    conn_params {'charset': 'utf8mb4',
           'client_flag': 2,
           'conv': {0: <class 'decimal.Decimal'>,
          1: <class 'int'>,
          2: <class 'int'>,
          3: <class 'int'>,
          4: <class 'float'>,
          5: <class 'float'>,
          7: <function DateTime_or_None at 0x7f6218e5b490>,
          8: <class 'int'>,
          9: <class 'int'>,
          10: <function Date_or_None at 0x7f6218e5b640>,
          11: <function typecast_time at 0x7f6219d803a0>,
          12: <function DateTime_or_None at 0x7f6218e5b490>,
          13: <class 'int'>,
          15: <class 'bytes'>,
          245: <class 'bytes'>,
          246: <class 'decimal.Decimal'>,
          249: <class 'bytes'>,
          250: <class 'bytes'>,
          251: <class 'bytes'>,
          252: <class 'bytes'>,
          253: <class 'bytes'>,
          254: <class 'bytes'>,
          <class 'array.array'>: <function array2Str at 0x7f6218e84160>,
          <class 'decimal.Decimal'>: <function Decimal2Literal at 0x7f6218e840d0>,
          <class 'datetime.date'>: <function Thing2Literal at 0x7f6218e84040>,
          <class 'datetime.datetime'>: <function DateTime2literal at 0x7f6218e5b6d0>,
          <class 'datetime.timedelta'>: <function DateTimeDelta2literal at 0x7f6218e5b760>,
          <class 'set'>: <function Set2Str at 0x7f6218e5bd90>,
          <class 'NoneType'>: <function None2NULL at 0x7f6218e5bf40>,
          <class 'int'>: <function Thing2Str at 0x7f6218e5be20>,
          <class 'float'>: <function Float2Str at 0x7f6218e5beb0>,
          <class 'bool'>: <function Bool2Str at 0x7f6218e5bc70>},
 'database': 'test-db',
 'password': 'test_password',
 'unix_socket': '/example/test-db',
 'user': 'example_user'}

Would be better if all db credentials in debug mode should be masked also with '*'.

[Tim Graham]

It's unclear how to reproduce the problem. Please reopen if you can provide a minimal example.

[bytej4ck]

Replying to Tim Graham:

It's unclear how to reproduce the problem. Please reopen if you can provide a minimal example.

My bad, I just edited the ticket.

[Jacob Walls]

Thanks, that was enough for me to reproduce the issue. I can see how this violates least surprise if these are substantially the same settings already masked elsewhere in the debug view.

The initial wontfix (comment:5:ticket:21098) in a related ticket for masking sensitive POST parameters argued masking wouldn't be worthwhile (leaks developer's own secrets to developer, DEBUG page is documented as always potentially leaking information), but ticket:21098 was eventually fixed once there was a simpler implementation.

Here is a potential tiny patch that I just confirmed fixes the issue, although it would be using @sensitive_variables() outside the context of a view, which I took to be the use case it was designed for:

django/db/backends/base/base.py

@@ -20 +20 @@
 from django.db.utils import DatabaseErrorWrapper, ProgrammingError
 from django.utils.asyncio import async_unsafe
 from django.utils.functional import cached_property
+from django.views.decorators.debug import sensitive_variables
 
 NO_DB_ALIAS = "__no_db__"
 RAN_DB_VERSION_CHECK = set()
@@ -235 +236 @@
     # ##### Backend-specific methods for creating connections #####
 
     @async_unsafe
+    @sensitive_variables("conn_params")
     def connect(self):
         """Connect to the database. Assume that the connection is closed."""
         # Check for invalid configurations.

django/views/debug.py

@@ -234 +234 @@
         if is_multivalue_dict:
             # Cleanse MultiValueDicts (request.POST is the one we usually care about)
             value = self.get_cleansed_multivaluedict(request, value)
+        else:
+            value = self.cleanse_setting("", value)
         return value
 
     def get_traceback_frame_variables(self, request, tb_frame):

[Natalia Bidart]

Thank you Jacob for the extra details, though I'm not being able to reproduce. Making connect fail does not help me get a traceback at runtime, it only prevents the development server to run.
Can someone please provide, ideally, a regression test? Or at least a code snippet for a view to make this more easily reproducible.

Side note: if we eventually accept this ticket, I think it should include some audit of other potential places where the connection params could be used and not marked as sensitive.

[Jacob Walls]

Yeah, the failure has to be "occasional" enough to not prevent the development server from booting. I know this is lazier than writing a unit test, but two quick reproducers:

• Start your db, boot the development server, stop the db, and visit another page that makes a db query.
• Or, while keeping your db up, fiddle with simulating a "random" failure like this:

django/db/backends/postgresql/base.py

@@ -181 +181 @@
     _named_cursor_idx = 0
     _connection_pools = {}
 
+    # Simulate an occasional failure.
+    denominator_will_become_zero = 5
+
     @property
     def pool(self):
         pool_options = self.settings_dict["OPTIONS"].get("pool")
@@ -308 +311 @@
         #   default when no value is explicitly specified in options.
         # - before calling _set_autocommit() because if autocommit is on, that
         #   will set connection.isolation_level to ISOLATION_LEVEL_AUTOCOMMIT.
+        self.denominator_will_become_zero -= 1
+        # Simulate occasional failure.
+        1 / self.denominator_will_become_zero
+        print(self.denominator_will_become_zero)
+
         options = self.settings_dict["OPTIONS"]
         set_isolation_level = False
         try:

[Baha Sdtbekov]

I think there's a deeper problem here.
We can have a lot of cases where we can leak classified data.

For example, using salted_hmac from crypto
If we specify wrong algorithm we can leak the secret key of django, I understand that this is because of wrong usage but I would like to hide them or not show them at all.

Because leaked data in production is a big loss.

[Baha Sdtbekov]

you can try to make something like SafeExceptionReporterFilter for local vars

but I am concerned about the performance of such a solution

[Natalia Bidart]

Replying to Jacob Walls:

Yeah, the failure has to be "occasional" enough to not prevent the development server from booting. I know this is lazier than writing a unit test, but two quick reproducers:

• Start your db, boot the development server, stop the db, and visit another page that makes a db query.
• Or, while keeping your db up, fiddle with simulating a "random" failure like this:

Thank you Jacob, I assumed so and I did try both of these before, but the bit that I was missing was to actually "go" to the DB to fetch or write something. In retrospect, this is obvious, but somehow I missed it. I managed to reproduce the issue, but as anticipated, I fear that this ticket, if accepted, has a scope beyond handling only connect. Specifically, if accepted, we should at least:

• mark as sensitive every location that uses the connection params via calling get_connection_params
• mark as sensitive settings_dict and conn_params in get_connection_params
• audit the rest of the code for other parts using or accessing the DB settings/password in any way

I'm not convinced this is within the scope of what Django core should handle. It has always been stated that ​error report filtering is a best-effort solution and further measures should be taken to sanitize or guard the error logs. Despite my reservations, I'm inclined to accept this, as it could provide a clear improvement by carefully adding the sensitive_variables decorator to the audited methods and functions.

Regarding Baha's comments, I think we should limit this ticket to "DB connection params" and avoid expanding the scope, as it’s better to focus on one specific issue. In summary, I'm accepting with the rationale/caveats expressed above.