Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Specific ASVS Item Addressing User Enumeration Vulnerabilities #1741

Closed
ImanSharaf opened this issue Oct 2, 2023 · 31 comments · Fixed by #1854
Closed

Add Specific ASVS Item Addressing User Enumeration Vulnerabilities #1741

ImanSharaf opened this issue Oct 2, 2023 · 31 comments · Fixed by #1854
Assignees
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet V2 _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@ImanSharaf
Copy link
Collaborator

The current ASVS items under category 2 don't explicitly address user enumeration vulnerabilities that arise from different responses given by the application during authentication attempts (e.g., "This user doesn't exist" vs. "Incorrect password").

Proposed Change
Add a new ASVS item under category 2 that explicitly addresses user enumeration during the authentication process.

Draft for the new ASVS item:

2.X.X Verify that the application provides generic error messages during the authentication process to prevent user enumeration vulnerabilities. Regardless of the authentication failure reason, responses such as "Invalid username or password" should be used.

Justification

User enumeration vulnerabilities allow attackers to determine if a username or email is registered with the service. This vulnerability can provide attackers with a starting point for brute force attacks, phishing campaigns, and other malicious activities targeting valid users. Addressing this in the ASVS will raise awareness among developers and security auditors about this often-overlooked vulnerability.

@elarlang
Copy link
Collaborator

elarlang commented Oct 2, 2023

What risk or attack scenario you would like to take down with the requirement?

@elarlang elarlang added the 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet label Oct 2, 2023
@ImanSharaf
Copy link
Collaborator Author

The primary risk that we are addressing with this requirement is the potential for an attacker to enumerate and identify valid usernames or user IDs within the application. This presents multiple concerns:

Phishing and Social Engineering: Armed with a list of valid usernames or emails, an attacker can craft more convincing phishing campaigns. By targeting actual users of a service, the success rate of phishing attempts can increase.

Data Leakage: In some scenarios, the very knowledge of a specific individual being a user of a certain application can be considered sensitive information. For example, being able to confirm that a particular email address is registered on a healthcare or financial site might leak personal data about that individual's affiliations or activities.

Chaining Vulnerabilities: User enumeration, while seemingly benign on its own, can be a part of a larger chain of vulnerabilities. An attacker leveraging multiple vulnerabilities can potentially cause more significant harm when they have knowledge of valid users.

@elarlang
Copy link
Collaborator

elarlang commented Oct 2, 2023

For me basically the only reasonable point is the "data leakage" example. Phising campaigns (in general) feels a bit more out of ASVS scope.

Nowadays it's widespread solution that username is email address, and there is plenty of ways to figure them out in general. Against mass-requests there are anti-automation rules.

If there is self-registration functionality and there is free form of choosing username - how do you provide the service without letting users know, that certain username is taken?

If username is an email address, maybe there is possibility to send some abstract message that "Check your email for activation".

(Genral feedback) I would like to see more precise and technical examples, "may cause more harm" does not carry any information for me.

@elarlang
Copy link
Collaborator

elarlang commented Oct 2, 2023

Vision from my side: I don't see it as requirement for improving authentication (proposed 2.X.X).

Only value here is information leakage (8.(1 or 3).X) - the fact that certain user/email is using the application. The problem is, you can not always hide it - maybe usernames or emails are diplayed somewhere by business logic rules - which also means we can not require hiding it for everyone and for every case.

To watch it as information leakage, I think it also covers other pointed out scenarios like phising, as this acticity just using the information leakage but it's not vector against the application itself.

@tghosth
Copy link
Collaborator

tghosth commented Oct 9, 2023

Hi @ImanSharaf, just a reminder to always search for issues before you open new ones :)

In this case, I gave an opinion similar to what @elarlang wrote above.

As @elarlang wrote above, I agree that this could be a data leakage/information disclosure issue where being able to discover the existence of a single user in an application (e.g. abortion clinic website) would be considered a significant breach of confidentiality/privacy.

Can you think of a relevant requirement text?

@tghosth tghosth added the _5.0 - prep This needs to be addressed to prepare 5.0 label Oct 9, 2023
@elarlang
Copy link
Collaborator

elarlang commented Oct 9, 2023

@tghosth - before requirement text, we should finding agreement and understanding, is it actually reachable. Like my comments here: #1741 (comment)

@csfreak92
Copy link
Collaborator

I agree with @elarlang on this one. Knowing someone's username is not really an attack vector or a vulnerability on its own. We had extensive discussions with colleagues and clients before and this is usually not an issue we report on a pentest as a finding unless we were able to chain multiple vulnerabilities (which are more impactful than username enumeration) that could lead to a user account being compromised. The rationality was anyone could figure out your username/email if it is let's say firstname.lastname@domain.org, so that doesn't really stop an attacker to enumerate users. What stops them are other controls behind that. And actually, some systems out there design it that way to prevent frustration from users whenever registering for a new account if there are generic error messages it gets confusing.

I feel like if ever we would like to focus on improving the authentication mechanisms, we should focus on other controls (such as account lockout to deter brute force attacks, enforcing MFA, etc).

@jmanico
Copy link
Member

jmanico commented Oct 15, 2023 via email

@elarlang
Copy link
Collaborator

Well, to the only defense against username-password authentication is to not use only username-password for authentication.

For comparison for Jim - if in Estonia you want log in to a internet bank or to some government related sites, you can not use username-password combination anywhere.

In general I agree, that if an attacker knows the username, it makes attacker's life easier, but I it's not trivial to build a requirement to require hiding it for everyone and for every case.

Email address as username is too widespread solution. Or Jim want to say, that authentication to Gmail is problematic as you use your email address as username?

@jmanico
Copy link
Member

jmanico commented Oct 15, 2023 via email

@elarlang
Copy link
Collaborator

Developers should be considering these options.

Yes, developers (or security analyst) should consider this as option. But at the moment I can not see the material for requirement, what everyone must do.

@belalena
Copy link

We have ASVS Requirement:
2.2.8. Verify that all failed authentication challenges respond in the same average response time.
This requirement emphasizes consistent response times in the event of failed authentication to avoid revealing a real user's login.

Simultaneously, there's a recommendation in the Authentication Cheat Sheet, which advises displaying a generic error message in different authentication scenarios, even when:

-The user ID or password is incorrect.
-The account doesn't exist.
-The account is locked or disabled.

BUT the current conversation is mostly about not wanting to use a standard message...
To deal with it, I see two options:

  1. Create a matching requirement about generic message.
  2. Think about removing the requirement (2.2.8) and the recommendation from the Cheat Sheet, which could be causing the conflict.

Does this make sense?

@elarlang
Copy link
Collaborator

BUT the current conversation is mostly about not wanting to use a standard message...

Current issue is not limited to username leakage from error messages, but in general - application should hide is some user registered there or not. Idea is nice but taking into account widespread solution to use email address as user-name, makes it hard to provide good and "fits for all" requirement for that.

@elarlang
Copy link
Collaborator

Maybe I provide context with some parallel issue.

Let's say application keeps files in public folder, those are accessible with HTTP request but actually require authorization. For the folder is available directory listing.

We have requirement for folder listing, as unintended information leakage:

  • V14.3.4 Verify that directory browsing is disabled unless deliberately desired.

If we disable folder listing, we don't fix authorization problem for accessing files. It just closes one (main) source of information which files are there, but the problem in the application still exists.

The same is with username enumeration from authentication perspective - practically hiding usernames won't compensate problems with authentication. At the same time, it does not make sense to make attackers life easier and do not reveal information which is not intended to be public.

If to use example of V14.3.4, maybe something like (from information leakage perspective):

  • V8.X.X - Verify that application does not reveal information about registered users to the application (such as name, username, email address) unless it is public information and accepted business logic decision.

@belalena
Copy link

Definitely! The very first message in this issue is about potential leakage of information if some user is registered in a system or not through the error messages.

Some potential places where the disclosure of whether a user is registered or not may occur:

  1. The response code (for instance 200, 403)
  2. The error message (Your password is not correct, Login is not correct)
  3. The response time (amount of time differs if user exists or not)

For the 3d one we have 2.2.8 requirement.

What about 1st and 2d?

What if we add them to 2.2.8?

2.2.8. Verify that all failed authentication challenges respond in the same way:

  • HTTP code,
  • average response time,
  • error message.

@elarlang
Copy link
Collaborator

I agree it makes sense to change 2.2.8 to cover more cases.

But now we kind of have 2 parallel topics: to hide the fact that this user is registered in the application at all and to hide what went wrong during authentication (2.2.8 improvement).

@belalena
Copy link

I can't think of any other reason to have 2.2.8 other than to hide the fact that a user with that login is already registered.
However, I also understand why it's challenging to implement a similar requirement during the registration of a new user.

In my view, 2.2.8. (after extending) would cover it for applications without a self-registration.
But to achieve this, the same requirements should be developed for Forgotten Password function (or adding Forgotten Password functionality to 2.2.8). For instance:
2.X.X. Verify that Forgotten Password functionality responds in the same way whether it's requested for an existing user or a non-existing user:

  • HTTP code,
  • average response time,
  • error message.

V8.X.X sounds good for me. It would help highlighting the problem of information leakage during registration in self-registration applications, among other ways of revealing of user information.

@elarlang
Copy link
Collaborator

In ASVS v3.0.1 there was requirement:
V2.18 Verify that information enumeration is not possible via login, password reset, or forgot account functionality.

@belalena
Copy link

belalena commented Oct 18, 2023

Since requirement 2.18 was removed due to deprecation (#236), I suggest removing requirement 2.2.8 as it makes no sense apart from 2.18

@elarlang
Copy link
Collaborator

Good investigation :) I agree, it does not make sense or give meaningful impact from authentication point of view and this is the argument from NIST.

Here I watch it as information leakage, which can be used against authentication attacks, but where mentioned (NIST, #236) MFA need to take away the attack vector.

@tghosth
Copy link
Collaborator

tghosth commented Oct 23, 2023

I agree that not using email as an email address is probably more of a "should do" rather than a "must do" so it isn't really relevant for ASVS.

I can only really repeat my comment from above: #1741 (comment)

I agree that this could be a data leakage/information disclosure issue where being able to discover the existence of a single user in an application (e.g. abortion clinic website) would be considered a significant breach of confidentiality/privacy.

Other than that, I don't think we need requirements for this.

@elarlang
Copy link
Collaborator

Other than that, I don't think we need requirements for this.

but we already have requirement for that and I agreed (#1741 (comment)) it can be finetuned like @belalena proposed in #1741 (comment) to cover it more widely than written at the moment.

# Description L1 L2 L3 CWE NIST §
2.2.8 [ADDED] Verify that all failed authentication challenges respond in the same average response time.

Other option is to delete it. Or is there any other good reason to have 2.2.8?

@tghosth
Copy link
Collaborator

tghosth commented Oct 31, 2023

Ok, I think I understand better now.

What if we were to expand 2.2.8 and bump it up to L3 as I think it is a hard requirement

# Description L1 L2 L3 CWE NIST §
2.2.8 [ADDED] Verify that valid users cannot be deduced from failed authentication challenges such as based on error messages, HTTP response codes or different response times. Registration and forgot password functionality should also have this protection.

@tghosth
Copy link
Collaborator

tghosth commented Jan 25, 2024

@ImanSharaf @belalena do you have any comments on the requirement proposal in the previous comment?

@elarlang
Copy link
Collaborator

Just a note that in the proposed requirement text, there is no closing bracket for the opening one.

@tghosth
Copy link
Collaborator

tghosth commented Jan 25, 2024

Thanks I fixed that bracket

@ImanSharaf
Copy link
Collaborator Author

@tghosth all good!

@elarlang elarlang added the 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR label Jan 30, 2024
@elarlang
Copy link
Collaborator

By content it's V8 requirement. Should we keep it in V2 together with authentication requirements, or should we move it to V8?

@elarlang elarlang removed the 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR label Jan 30, 2024
@tghosth
Copy link
Collaborator

tghosth commented Feb 2, 2024

I think we should keep it as V2 because not everything in the requirement is strictly error related

@elarlang
Copy link
Collaborator

elarlang commented Feb 2, 2024

V8 is sensitive data. The fact that the user is registered to a site, can be considered as sensitive information.

@tghosth
Copy link
Collaborator

tghosth commented Feb 2, 2024

Sorry, I got confused with V7 😂😂.

Nevertheless I think it should stay with AuthN because it is quite specific

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet V2 _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants