Discourage logging f-strings due to security considerations

[ariebovenberg]

BPO	46200
Nosy	@rhettinger, @gpshead, @vsajip, @ericvsmith, @Tinche, @ariebovenberg

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2021-12-30.07:44:15.228>
labels = ['3.8', '3.9', '3.7', 'docs', 'type-security', '3.10', '3.11', 'library']
title = 'Discourage logging f-strings due to security considerations'
updated_at = <Date 2022-02-19.12:27:19.966>
user = 'https://github.com/ariebovenberg'

bugs.python.org fields:

activity = <Date 2022-02-19.12:27:19.966>
actor = 'ariebovenberg'
assignee = 'docs@python'
closed = False
closed_date = None
closer = None
components = ['Documentation', 'Library (Lib)']
creation = <Date 2021-12-30.07:44:15.228>
creator = 'ariebovenberg'
dependencies = []
files = []
hgrepos = []
issue_num = 46200
keywords = []
message_count = 13.0
messages = ['409352', '409416', '409418', '409495', '409499', '409505', '412357', '412473', '412474', '412496', '412504', '413519', '413547']
nosy_count = 7.0
nosy_names = ['rhettinger', 'gregory.p.smith', 'vinay.sajip', 'eric.smith', 'docs@python', 'tinchester', 'ariebovenberg']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue46200'
versions = ['Python 3.7', 'Python 3.8', 'Python 3.9', 'Python 3.10', 'Python 3.11']

[ariebovenberg]

(I've already submitted this issue to security@python.org, who directed me to propose a documentation change here)

Logging f-strings is quite popular, because the % style of logging is often considered ugly.
However, logging preformatted strings comes with security risks which should explicitly be mentioned in the logging documentation.

The following example illustrates the problem:

    logger.info('look: %s', untrusted_string)                # OK
    logger.info('look: %(foo)s', {'foo', untrusted_string})  # OK
    logger.info(f'look: {untrusted_string}')                 # OK (security-wise)
    logger.info(f'look: {untrusted_string}', some_dict)      # DANGER!

The problem is that untrusted_string will be interpreted as a string template by the logger.
If untrusted_string has the value %(foo)999999999s (where foo is present in some_dict),
logging gets stuck trying to add over a gigabyte of whitespace to a string.
In other words: a Denial-of-Service attack.

Of course, this combination of f-string and logger arguments is unlikely. But it is plausible that:

• After a refactoring to f-string, removing the dict argument is forgotten;
• A developer adding a variable to a message prefers using f-strings and thus creates a template mixing the two styles (e.g. {foo} and %(bar)s);
• The string is passed through a logging Filter or other function which adds a dict argument.

Open questions:

1. Do we want to simply highlight the risks, or actively discourage logging f-strings?

   My thoughts:
   I feel it's important enough to actively discourage logging preformatted strings (i.e. also .format and manual %-formatting).
   A strong recommendation will allow security linters to flag this.
   Additionally, there are other advantages to the formatting built into logging (e.g. performance).

2. Should we type annotate logger msg as Literal[str] following PEP-675?

   My thoughts:
   Annotating msg as a literal string will greatly help enforce this best practice.
   The adoption of typecheckers exceeds that of security linters.

3. Where should this risk be documented?
   a. In the logger.debug function docs
   b. In the logging docs introduction (like xml.etree.elementtree)
   c. In a new "security" section of the logging docs
   d. An informational PEP on logging security considerations (similar to PEP-672)

My thoughts:
I fear option (a) would not get the attention of most readers.
Option (c) or (d) may be useful to also highlight general risks
of logging (i.e. log injection)

As soon as there is agreement on these questions, I would be happy to submit a PR.