Opened 5 days ago

Last modified 3 minutes ago

#36179 new Bug

hexed strings in common passwords database are not handled

Reported by: Michel Le Bihan Owned by: Michel Le Bihan
Component: contrib.auth Version: 5.1
Severity: Normal Keywords: CommonPasswordValidator
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no
Pull Requests:19155 build:success

Description (last modified by Michel Le Bihan)

Hello,

The common passwords database file (https://github.com/django/django/blob/main/django/contrib/auth/common-passwords.txt.gz) used by CommonPasswordValidator contains hexed strings like $hex[617364666a6b6c3a] on line 1679. That decodes to asdfjkl: which I believe is a common password that was intended to be included in the database. Another example is $hex[2623323333363a] on line 8616 that decodes to &#2336:. I see that https://gist.github.com/roycewilliams/226886fd01572964e1431ac8afc999ce contains the line 50334:72aff1cfd90a90fd4174eb6dfdff5df7bbbe7e5b:$HEX[617364666a6b6c3a] and echo -n 'asdfjkl:' | sha1sum produces 72aff1cfd90a90fd4174eb6dfdff5df7bbbe7e5b. CommonPasswordValidator does not handle those hexed strings which I believe is wrong.

I propose to update the database file to decode the hexed values and remove those that obviously can't be entered by a user.

According to the ticket's flags, the next step(s) to move this issue forward are:

  • To improve the patch as described in the pull request review comments or on this ticket, then uncheck "Patch needs improvement".
  • If creating a new pull request, include a link to the pull request in the ticket comment when making that update. The usual format is: [https://github.com/django/django/pull/#### PR].

Change History (10)

comment:1 by Michel Le Bihan, 5 days ago

Description: modified (diff)

comment:2 by Michel Le Bihan, 5 days ago

Has patch: set

comment:3 by Antoliny, 4 days ago

Owner: set to Michel Le Bihan
Status: newassigned

comment:4 by Sarah Boyce, 4 days ago

Resolution: invalid
Status: assignedclosed

Thank you for the ticket
I think you are referring to there passwords asdfjkl; and ठ (; not :) which are validating correctly, the others are not in the list

comment:5 by Michel Le Bihan, 4 days ago

Hello,

I'm referring to those with : at the end. Please just search $hex in that file and you will notice the issue.

comment:6 by Michel Le Bihan, 4 days ago

Resolution: invalid
Status: closednew

comment:8 by Sarah Boyce, 4 days ago

Triage Stage: UnreviewedAccepted

Ah thank you!
Folks can de-hex using this tool: https://www.rapidtables.com/convert/number/hex-to-ascii.html

Here is a regression test:

  • TabularUnified tests/auth_tests/test_validators.py

    a b class CommonPasswordValidatorTest(SimpleTestCase):  
    273273            CommonPasswordValidator().validate("godzilla")
    274274        self.assertEqual(cm.exception.messages, [expected_error])
    275275
     276    def test_common_hexed_codes(self):
     277        expected_error = "This password is too common."
     278        common_hexed_passwords = ["asdfjkl:", "&#2336:"]
     279        for password in common_hexed_passwords:
     280            with self.subTest(password=password):
     281                with self.assertRaises(ValidationError) as cm:
     282                    CommonPasswordValidator().validate(password)
     283                self.assertEqual(cm.exception.messages, [expected_error])
     284
    276285    def test_validate_custom_list(self):
    277286        path = os.path.join(

comment:9 by Michel Le Bihan, 4 days ago

I sent a patch that unhexes the values in the database: https://github.com/django/django/pull/19155

comment:10 by Raphael Gaschignard, 3 minutes ago

Patch needs improvement: set

While most of the $hex entries are removed, there's still one entry that's just $hex. I _believe_ this is an oversight, and not intentional.

Note: See TracTickets for help on using tickets.
Back to Top