Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-70371

Define a password length upper limit to future-proof from hashing denial of service risks



    • Affected Branches:


      We should mitigate any future risks of long password denial of service.

      After doing a deep dive into how password resets work in our various authentication plugins, this does not appear to be a current security risk.

      All authentication plugins except for OAuth use moodlelib's hash_internal_user_password method, which utilises PHP's password_hash function using the default hashing algorithm. Currently, that is bcrypt, which (as mentioned in the docs) truncates passwords after 72 characters automatically and therefore effectively mitigates this risk. I confirmed this was the case in practice (both via forgot password and preferences).

      I would suggest that we create a constant containing a reasonable upper limit (as is recommened by OWASP), which is applied as part of hash_internal_user_password(), so that when, in future, PHP changes its default hashing algorithm, it does not inadvertently introduce a login DoS vulnerability into Moodle.

      Regarding OAuth authentication plugin password resets, the hashing algorithms used are currently either MD5, SHA1 or none, and according to the same OWASP docs, using "faster" hashing algorithms to mitigate length concerns is a valid use case, therefore this does not appear to pose a denial of service vulnerability. I would suggest we also look into whether it would be suitable to update that with a limit while we are making these changes.

      I have marked this as an improvement, but think we should consider backporting this to any relevant security branches, if it does not raise any breaking changes/blockers that would prevent it.


      Writeup/testing/investigation by Michael Hawkins, after receiving an initial report re potential login DoS on forgot password reset by Royniss  (SF case 00091645).




            Unassigned Unassigned
            michaelh Michael Hawkins
            Component watchers:
            Adrian Greeve, Jake Dallimore, Mathew May, Mihail Geshoski, Peter Dias, Sujith Haridasan
            0 Vote for this issue
            1 Start watching this issue