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

    XMLWordPrintable

    Details

    • Affected Branches:
      MOODLE_311_STABLE

      Description

      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).

        Attachments

          Activity

            People

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

              Dates

              Created:
              Updated: