Opened 4 weeks ago

Last modified 4 weeks ago

#35930 new Cleanup/optimization

Database password visible on debug page in local variable

Reported by: bytej4ck Owned by: Baha Sdtbekov
Component: Error reporting Version: dev
Severity: Normal Keywords: db, password, exposed
Cc: bytej4ck Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by 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 '*'.

Attachments (1)

2024-11-22_21-17.png (88.1 KB ) - added by bytej4ck 4 weeks ago.

Download all attachments as: .zip

Change History (12)

by bytej4ck, 4 weeks ago

Attachment: 2024-11-22_21-17.png added

comment:1 by Tim Graham, 4 weeks ago

Component: UncategorizedError reporting
Resolution: needsinfo
Status: newclosed
Type: UncategorizedBug

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

comment:2 by bytej4ck, 4 weeks ago

Description: modified (diff)
Resolution: needsinfo
Status: closednew
Summary: Database password visible on debug page (view source only)Database password visible on debug page
Version: 4.1

in reply to:  1 comment:3 by bytej4ck, 4 weeks ago

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.

comment:4 by Jacob Walls, 4 weeks ago

Summary: Database password visible on debug pageDatabase password visible on debug page in local variable

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

    diff --git a/django/db/backends/base/base.py b/django/db/backends/base/base.py
    index e6e0325d07..b950c20350 100644
    a b from django.db.transaction import TransactionManagementError  
    2020from django.db.utils import DatabaseErrorWrapper, ProgrammingError
    2121from django.utils.asyncio import async_unsafe
    2222from django.utils.functional import cached_property
     23from django.views.decorators.debug import sensitive_variables
    2324
    2425NO_DB_ALIAS = "__no_db__"
    2526RAN_DB_VERSION_CHECK = set()
    class BaseDatabaseWrapper:  
    235236    # ##### Backend-specific methods for creating connections #####
    236237
    237238    @async_unsafe
     239    @sensitive_variables("conn_params")
    238240    def connect(self):
    239241        """Connect to the database. Assume that the connection is closed."""
    240242        # Check for invalid configurations.
  • django/views/debug.py

    diff --git a/django/views/debug.py b/django/views/debug.py
    index 10b4d22030..67e3f3d8b0 100644
    a b class SafeExceptionReporterFilter:  
    234234        if is_multivalue_dict:
    235235            # Cleanse MultiValueDicts (request.POST is the one we usually care about)
    236236            value = self.get_cleansed_multivaluedict(request, value)
     237        else:
     238            value = self.cleanse_setting("", value)
    237239        return value
    238240
    239241    def get_traceback_frame_variables(self, request, tb_frame):
Last edited 4 weeks ago by Jacob Walls (previous) (diff)

comment:5 by Baha Sdtbekov, 4 weeks ago

Owner: set to Baha Sdtbekov
Status: newassigned

comment:6 by Natalia Bidart, 4 weeks ago

Resolution: needsinfo
Status: assignedclosed
Version: dev

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.

comment:7 by Jacob Walls, 4 weeks ago

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

    diff --git a/django/db/backends/postgresql/base.py b/django/db/backends/postgresql/base.py
    index c864cab57a..8f653f0078 100644
    a b class DatabaseWrapper(BaseDatabaseWrapper):  
    181181    _named_cursor_idx = 0
    182182    _connection_pools = {}
    183183
     184    # Simulate an occasional failure.
     185    denominator_will_become_zero = 5
     186
    184187    @property
    185188    def pool(self):
    186189        pool_options = self.settings_dict["OPTIONS"].get("pool")
    class DatabaseWrapper(BaseDatabaseWrapper):  
    308311        #   default when no value is explicitly specified in options.
    309312        # - before calling _set_autocommit() because if autocommit is on, that
    310313        #   will set connection.isolation_level to ISOLATION_LEVEL_AUTOCOMMIT.
     314        self.denominator_will_become_zero -= 1
     315        # Simulate occasional failure.
     316        1 / self.denominator_will_become_zero
     317        print(self.denominator_will_become_zero)
     318
    311319        options = self.settings_dict["OPTIONS"]
    312320        set_isolation_level = False
    313321        try:

comment:8 by Jacob Walls, 4 weeks ago

Resolution: needsinfo
Status: closednew

comment:9 by Baha Sdtbekov, 4 weeks ago

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.

Version 1, edited 4 weeks ago by Baha Sdtbekov (previous) (next) (diff)

comment:10 by Baha Sdtbekov, 4 weeks ago

you can try to make something like SafeExceptionReporterFilter for local vars

but I am concerned about the performance of such a solution

in reply to:  7 comment:11 by Natalia Bidart, 4 weeks ago

Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

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.

Note: See TracTickets for help on using tickets.
Back to Top