AdminSite views (such as login) leak sensitive POST data

[Olivier Dalang]

Hey !

Recently I came accross plain text user passwords in debug reports when there is an exception on the login view. Such exceptions can happen very easily in many basic scenarios such as an unreachable database. The passwords are visible in:

1/ regular debug reports when DEBUG=True

https://code.djangoproject.com/attachment/ticket/36542/image.png

2/ email reports when DEBUG=False and include_html=True

https://code.djangoproject.com/attachment/ticket/36542/image%20(1).png

I reported this to the security team, but quite reasonably they decided it's not a security issue since it's very clear no one should use DEBUG=True in PROD (and there, users would just see their own password anyway), and that the docs already warn quite a bit against enabling "include_html" in email reports. Yet they agreed this is still worth improving.

For context, Django already redacts much less critical infos by default in the report such as the csrftoken or internal settings, even if these are harder to exploit and unlike user passwords usually already known to admins. Having such info redacted gives some false sense of security about these reports.

I didn't test but think there are other cases in django core such as user creation forms (password1, password2) or password changes (old_password, new_password1, new_password2), and of course this would also happen in third party apps / user code.

In terms of fixing this, why don't we just apply the same filter used for settings (API|AUTH|TOKEN|KEY|SECRET|PASS|SIGNATURE|HTTP_COOKIE) to POST parameters as well as variables in the full trace ? I feel this would cover most cases and be quite straightforward to implement and to understand for users. For that matter, better to redact too many variables than too few.

Cheers !

[Sarah Boyce]

Thank you for raising
I have updated the ticket description to reflect the current bug

In terms of fixing this, why don't we just apply the same filter used for settings (API|AUTH|TOKEN|KEY|SECRET|PASS|SIGNATURE|HTTP_COOKIE) to POST parameters as well as variables in the full trace ? I feel this would cover most cases and be quite straightforward to implement and to understand for users. For that matter, better to redact too many variables than too few.

This requires more discussion as it's a change in behavior and some folks are likely to want to see some POST data for debugging.

[Ankan Giri]

Hi! I’m interested in working on this issue.
I have experience working with Django, though I’m new to contributing to Django itself.
Thanks!

[kanin.kearpimy@…]

Hi,

I noticed there is no updating for four months. If no one is working on this, Can I take one?

[kanin.kearpimy@…]

Hello,

I open patch to this issue ​https://github.com/django/django/pull/20959.

Screenshot (seem like debug page only has light mode)

[Tim McCurrach]

I’ve just taken a look at the PR, and I think it’s worth expanding on a few things in this ticket for the sake of clarity:

• If DEBUG is False, both sensitive variables and sensitive post parameters are always cleaned/redacted from debug-reports. This holds, even when include_html=True.
• As far as I can tell, all of the built-in auth views are decorated with the appropriate sensitive_post_parameters decorators.
• The admin login/user views use the aforementioned built-in views for most of the heavy lifting, and so are protected too. (This is because even if the “top-level” view isn’t decorated, sensitive_post_parameters gets set on the request further down - which is all that matters).
• Most of the associated authentication functions further down the stack are decorated with the appropriate sensitive_variables decorators.

The above is to say, that all-in-all, even with include_html=True everything is actually already pretty well locked down. The reason the password appears in the screenshot above is that some of the functions deeper down the stack (at the auth-backend / model level) aren’t decorated with the sensitive_variables decorator.

I wonder if the docs are potentially misleading on this. They say:

The include_html argument of AdminEmailHandler is used to control whether the traceback email includes an HTML attachment containing the full content of the debug web page that would have been produced if DEBUG were True.

This is maybe a little bit ambiguous. My reading of the above would be that variables marked as sensitive would remain unfiltered - but that isn't true.

---

Regarding the original suggestion:

why don't we just apply the same filter used for settings (API|AUTH|TOKEN|KEY|SECRET|PASS|SIGNATURE|HTTP_COOKIE) to POST parameters as well as variables in the full trace ?

This won’t actually help the POST parameters since they are already filtered when DEBUG is False. It would cover a few more of the sensitive variables, but I think we could (maybe should) patch up the last few remaining functions in the auth app, that have any local variables that are password-y in them with the sensitive_variables decorator. Doing a quick search there don’t seem to be too many. That way, we still have all the local variable data for debugging.

[Tim McCurrach]

The other thing that might help is to provide clearer guidance in the docs about what makes include_html dangerous. Taking a quick look at techincal_500.html and techincal_500.txt - the only difference that jumps out at me is that the former includes local variables.

The docs mention this:

The reporter_class argument of AdminEmailHandler allows providing an django.views.debug.ExceptionReporter subclass to customize the traceback text sent in the email body.

We could also provide a simple example of how that is done if users wanted to have a more nuclear/blanket approach to filtering local variables.

[Jacob Walls]

The reason the password appears in the screenshot above is that some of the functions deeper down the stack (at the auth-backend / model level) aren’t decorated with the sensitive_variables decorator.

Noting we have an accepted ticket for that in #35930 with respect to the methods for initiating a database connection.

[kanin.kearpimy@…]

Hi,

Let me sum it up and please correct me if something doesn't make sense. So, now the problem is we doesn't have sensitive_post_parameters and sensitive_variables to cover sensitive function parameters, in this case password-related for POST request. That's why in DEBUG=True, there is password-related field appear?

Regarding below

why don't we just apply the same filter used for settings (API|AUTH|TOKEN|KEY|SECRET|PASS|SIGNATURE|HTTP_COOKIE) to POST parameters as well as variables in the full trace ?

it may doesn't help to broadly filter password in POST, because deeper down the stack still isn't covered by sensitive_post_parameters OR sensitive_variables as Tim mentioned here

This won’t actually help the POST parameters since they are already filtered when DEBUG is False. It would cover a few more of the sensitive variables, but I think we could (maybe should) patch up the last few remaining functions in the auth app, that have any local variables that are password-y

As Jacob shared there was a PR did such similar thing before in #35930

Noting we have an accepted ticket for that in #35930 with respect to the methods for initiating a database connection.

So, it's good idea to imitate the #35930 approach and cover up all auth app function with local password-y varialbes?

[Tim Schilling]

@Tim, it looks like the documentation you're looking for exists at ​https://docs.djangoproject.com/en/6.0/topics/logging/#adminemailhandler

The built-in AdminEmailHandler deserves a mention in the context of security. If its include_html option is enabled, the email message it sends will contain a full traceback, with names and values of local variables at each level of the stack, plus the values of your Django settings (in other words, the same level of detail that is exposed in a web page when DEBUG is True).

@kanin, I agree with your conclusion of "So, it's good idea to imitate the #35930 approach and cover up all auth app function with local password-y variables?". This seems like the better approach since it highlights on the view which parameters need to be sanitized because the decorator is physically close to it.

[Tim McCurrach]

Replying to Tim Schilling:

@Tim, it looks like the documentation you're looking for exists at ​https://docs.djangoproject.com/en/6.0/topics/logging/#adminemailhandler

Nice, that's exactly what I was suggesting. Thanks.

[kanin.kearpimy@…]

Hi everyone,

I found interesting bug for reset password. Well, this card relates to login mainly but reporter left comments of reset password scenario as well. So, I have a look at it.

Please see diagram below for clarification.

For login scenario, it's simpler, because when user hit login, it would invoke authenticate function to auth such user a credential. If database throw error, we can use sensitive_variables("password") to hide password-y local variables in relating methods which I did in current [PR](​https://github.com/django/django/pull/20959).

However, reset password scenario is another story. because only authenticating user is able to reset their password, system attempts to get user information via get_user. If database throw error there, it will directly go to debug page, no going to authenticate or any functions holding password-y local variables. So, sensitive_variables("password") doesn't work out this scenario (I tried added once, it doesn't work).

Technically, we can hide it in debug.py or need time to investigate more. But I'm not sure it will be too complicated for this ticket (or we can open it as another issue)?

Please suggest