thunderbird / thunderbird-android

K-9 Mail – Open Source Email App for Android
https://k9mail.app/
Apache License 2.0
9.97k stars 2.47k forks source link

Private information leak into debug log #5684

Open suuuehgi opened 2 years ago

suuuehgi commented 2 years ago

Describe the bug Full contact detail are present within the debug log.

09-20 17:13:45.489 17669 19640 V RealImapConnection: From: John Doe john@doe.com 09-20 17:13:45.489 17669 19640 V RealImapConnection: To: jane@doe.com

To Reproduce Steps to reproduce the behavior:

  1. Enable debug logging
  2. Follow the guide

Expected behavior Don't log that information or obfuscate it.

Environment (please complete the following information):

life777eternal commented 2 years ago

I have noticed this as well, and it takes some time to remove all the personal information with find and replace on Notepad Plus Plus.

parkerfath commented 2 years ago

@cketti - I was looking at this one and had some ideas. It seems like this is because the response from the IMAP server in RealImapConnection.readResponse() contains the headers, and headers contain senders' address, name, subject, etc. Arguably these are "sensitive" as suuuehgi states. So we could consider:

  1. Treating all the IMAP responses in readResponse as sensitive, and only log them if the "Log sensitive information" setting is enabled. (The downside would be that the only way to log IMAP responses would now be to log all sensitive info.)
  2. Treating all the IMAP responses as a "new" kind of sensitive, and add a new setting for "Log IMAP responses, including headers" (and maybe default it to "off"?)
  3. Doing some kind of regex on it and mask the email addresses?

I suppose it hinges largely on where we want to draw the line on what's "sensitive." Seems like there are probably other parts of the app one could want to debug without revealing all of their email details to the LogCat gods. What do you think?

cketti commented 2 years ago

I had a much more comprehensive approach in mind. We probably shouldn't log e.g. account names and folder names either (unless "log sensitive information" is enabled).

I guess it would be useful to have something like this:

SpecialLogger.d("Performing some operation on %{sensitive}s:%{sensitive}s", account, folderName)

The protocol logging is one of the most useful tools we have to remotely diagnose issues. So ideally we find a way to keep it enabled by default without logging sensitive information. Since we only output the parsed result, we could build some logging logic with special handling for some responses. The header.fields contents of the FETCH response are rarely important. So we could replace it with something like string[lines=4] in the log. In LIST responses we could replace all letters with x, but keep the special characters. That should still leave enough information to diagnose most issues.