Replace the call to re.findall with re.sub in _mask_credentials so matched values are not treated as regex patterns #413

ThePumpingLemma · 2020-04-07T14:18:57Z

This fixes #410

This also fixes a DoS attack:

Form is submitted with multiple empty fields that match the mask filter
Substitution loop repeatedly calls re.sub with an empty pattern
Cleansed string is inserted between every pair of characters
Loop iterations causes exponential growth of body string, preventing the request from ever finishing

matched values are not treated as regex patterns This fixes jazzband#410

barm

Can you add some more test cases to handle all the nuance here?

barm · 2020-04-07T14:37:56Z

project/tests/test_sensitive_data_in_request.py

@@ -36,6 +36,9 @@ def test_mask_credentials_handles_prefixes(self):
    def test_mask_credentials_handles_suffixes(self):
        self.assertNotIn("secret", self._mask("username-with-suffix=secret"))

+    def test_mask_credentials_handles_regex_characters(self):
+        self.assertNotIn("secret", self._mask("password=secret++"))


nasirhjafri

Thanks @ThePumpingLemma LGTM 👍

Replace the call to re.findall with re.sub in _mask_credentials so

72b80da

matched values are not treated as regex patterns This fixes jazzband#410

barm reviewed Apr 7, 2020

View reviewed changes

rossmechanic approved these changes Apr 7, 2020

View reviewed changes

nasirhjafri approved these changes Apr 7, 2020

View reviewed changes

nasirhjafri merged commit 186cfb3 into jazzband:master Apr 7, 2020

ThePumpingLemma deleted the say-no-to-regexes branch April 15, 2020 22:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace the call to re.findall with re.sub in _mask_credentials so matched values are not treated as regex patterns #413

Replace the call to re.findall with re.sub in _mask_credentials so matched values are not treated as regex patterns #413

ThePumpingLemma commented Apr 7, 2020 •

edited

Loading

barm left a comment

barm Apr 7, 2020

nasirhjafri left a comment

Replace the call to re.findall with re.sub in _mask_credentials so matched values are not treated as regex patterns #413

Replace the call to re.findall with re.sub in _mask_credentials so matched values are not treated as regex patterns #413

Conversation

ThePumpingLemma commented Apr 7, 2020 • edited Loading

barm left a comment

Choose a reason for hiding this comment

barm Apr 7, 2020

Choose a reason for hiding this comment

nasirhjafri left a comment

Choose a reason for hiding this comment

ThePumpingLemma commented Apr 7, 2020 •

edited

Loading