hypothesis / product-backlog

Where new feature ideas and current bugs for the Hypothesis product live
117 stars 7 forks source link

Security vulnerability - V-001 Minimum Password Length Too Low - `Medium` #1523

Closed indigobravo closed 8 months ago

indigobravo commented 10 months ago

Overview

Impacts

Description

The following is taken from the Draft SubGraph 2023 Security Review

Discussion

The minimum password length for users of h who create accounts is 2 characters. This issue was discovered in the source code and verified in the deployed application through the creation of an account with a 2 character password. The source code indicates that the developers were at some point aware of this issue but have not implemented a fix to date.

The issue can be seen in the public repository here: PASSWORD_MIN_LENGTH = 2 # FIXME: this is ridiculous

Note that this issue affects the h service when deployed using its own internal identity store, as is the case with the public service. Deployments that use external identity providers would not be affected, as those organizations can and will have their own password and authentication policies.

Impact Analysis

Users of the h application are not barred from creating accounts with extremely weak passwords. Since usernames are trivial to gather due to visibility as public annotators, attacks against users who have opted for simple passwords seem plausible.

Remediation Recommendations

This issue will require a change to the code to implement greater minimum complexity. Beyond this, there is the issue of users who currently have very low complexity passwords. Those users could be identified through a password strength exercise and then encouraged or forced to do a password change upon login. Something slightly similar was done in the past when the password hashing mechanism was improved, as evidenced by this check, though it would be a little more challenging as user interaction could be required, and the information that a user is affected would need to be stored somewhere.

dwhly commented 9 months ago

@indigobravo Lets tackle minimum length here and punt complexity to another card-- can you create? My suggestion on minimum length is 8 per NIST requirements. Enforced moving fwd, not on existing. Thoughts?

indigobravo commented 9 months ago

Enforced moving fwd, not on existing. Thoughts?

I think this has to be the way. We would break existing users unless we could come up with a smart way to make them change their password on next login.

indigobravo commented 9 months ago

Lets tackle minimum length here and punt complexity to another card-- can you create?

Done. New issue can be found here: