Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use only pure f-strings for logs, avoid %s/%r formatting #772

Merged
merged 1 commit into from
May 14, 2021

Conversation

nolar
Copy link
Owner

@nolar nolar commented May 14, 2021

This is secure, as Python checks for the existence of args/kwargs in logging before doing the printf-style formatting — see logging.LogRecord.getMessage():

msg = str(self.msg)
if self.args:
    msg = msg % self.args

If we do not pass the args/kwargs, the formatting will not be invoked, so injections of format specifiers via user-side resource fields are not possible — they will be logged "as is".

This is secure, as Python checks for the existence of args/kwargs in `logging` before doing the printf-style formatting — see `logging.LogRecord.getMessage()`:

```python
msg = str(self.msg)
if self.args:
    msg = msg % self.args
```

If we do not pass the args/kwargs, the formatting will not be invoked, so injections of format specifiers via user-side resource fields are not possible — they will be logged "as is".

Signed-off-by: Sergey Vasilyev <nolar@nolar.info>
@nolar nolar added the refactoring Code cleanup without new features added label May 14, 2021
@nolar nolar enabled auto-merge May 14, 2021 08:09
@nolar nolar merged commit 8d8059f into main May 14, 2021
@nolar nolar deleted the security-only-fstrings branch May 14, 2021 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Code cleanup without new features added
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant