rspec/rules/S5804/java/rule.adoc

include::../description.adoc[]

include::../ask-yourself.adoc[]

include::../recommended.adoc[]

== Sensitive Code Example

In a Spring-security web application the username leaks when:

* The string used as argument of https://docs.spring.io/spring-security/site/docs/current/api/org/springframework/security/core/userdetails/UserDetailsService.html[loadUserByUsername] method is used in an exception message:

----
public String authenticate(String username, String password) {
  // ....
  MyUserDetailsService s1 = new MyUserDetailsService();
  MyUserPrincipal u1 = s1.loadUserByUsername(username);

  if(u1 == null) {
    throw new BadCredentialsException(username+" doesn't exist in our database"); // Sensitive
  }
  // ....
}
----

* https://docs.spring.io/spring-security/site/docs/3.0.x/apidocs/org/springframework/security/core/userdetails/UsernameNotFoundException.html[UsernameNotFoundException] is thrown (except when it is in the loadUserByUsername method):

----
public String authenticate(String username, String password) {
  // ....
  if(user == null) {
      throw new UsernameNotFoundException("user not found"); // Sensitive
  }
  // ....
}
----

* https://docs.spring.io/spring-security/site/docs/4.0.x/apidocs/org/springframework/security/authentication/dao/AbstractUserDetailsAuthenticationProvider.html#setHideUserNotFoundExceptions-boolean-[HideUserNotFoundExceptions] is set to false:

----
DaoAuthenticationProvider daoauth = new DaoAuthenticationProvider();
daoauth.setUserDetailsService(new MyUserDetailsService());
daoauth.setPasswordEncoder(new BCryptPasswordEncoder()); 
daoauth.setHideUserNotFoundExceptions(false); // Sensitive
builder.authenticationProvider(daoauth);
----

== Compliant Solution

In a Spring-security web application:

* the same message should be used regardless of whether it is the wrong user or password:

[source,java]
----
public String authenticate(String username, String password) throws AuthenticationException {
  Details user = null;
  try {
    user = loadUserByUsername(username);
  } catch (UsernameNotFoundException | DataAccessException e) {
    // Hide this exception reason to not disclose that the username doesn't exist
  }
  if (user == null || !user.isPasswordCorrect(password)) {
     // User should not be able to guess if the bad credentials message is related to the username or the password
    throw new BadCredentialsException("Bad credentials");
  }
}
----

* https://docs.spring.io/spring-security/site/docs/4.0.x/apidocs/org/springframework/security/authentication/dao/AbstractUserDetailsAuthenticationProvider.html#setHideUserNotFoundExceptions-boolean-[HideUserNotFoundExceptions] should be set to true:

[source,java]
----
DaoAuthenticationProvider daoauth = new DaoAuthenticationProvider();
daoauth.setUserDetailsService(new MyUserDetailsService());
daoauth.setPasswordEncoder(new BCryptPasswordEncoder()); 
daoauth.setHideUserNotFoundExceptions(true); // Compliant
builder.authenticationProvider(daoauth);
----

include::../see.adoc[]

ifdef::env-github,rspecator-view[]

'''
== Implementation Specification
(visible only on this page)

include::../message.adoc[]

'''
== Comments And Links
(visible only on this page)

=== on 24 Sep 2020, 11:50:00 Eric Therond wrote:
In the future this rule can be improved:

* to check if the message sent to the user related to UsernameNotFoundException doesn't disclose any information about the existence of the user:

----
try {    
   userDetails = userDetailsService.loadUserByUsername(username);    
} catch (UsernameNotFoundException e) {    
   response.sendError(HttpServletResponse.SC_UNAUTHORIZED, e.getMessage());    
   return;    
}
----

* to check if the message sent to the user related to UsernameNotFoundException is the same than when the password is not good:

----
if (user == null) {
      throw new UsernameNotFoundException("Bad username");
}
if (!user.isPasswordCorrect(password)) {
      throw new BadCredentialsException("Bad credentials");
}
----

* Some https://www.baeldung.com/spring-security-custom-authentication-failure-handler[authentificationfailurehandler] like https://docs.spring.io/spring-security/site/docs/4.2.15.RELEASE/apidocs/org/springframework/security/web/authentication/ExceptionMappingAuthenticationFailureHandler.html[ExceptionMappingAuthenticationFailureHandler] seems to https://www.programcreek.com/java-api-examples/?code=helloworldtang%2Fspring-boot-cookbook%2Fspring-boot-cookbook-master%2Fapp%2Fsrc%2Fmain%2Fjava%2Fcom%2Ftangcheng%2Fapp%2Fapi%2Frest%2Fconfig%2FSecurityConfig.java[forward] the user to a page different based on exceptions and so disclose users existence. 





=== on 25 Sep 2020, 08:47:50 Eric Therond wrote:
I added again the case to raise when _UsernameNotFoundException_ is thrown because it causes an unnecessary  risk, there are several compliant solutions:

* using the same error message among all other exceptions
* using _UsernameNotFoundException_ but hiding it (_setHideUserNotFoundExceptions_ to false, so if spring allows us to hide these exceptions it is likely because they might be dangerous)
* not using _UsernameNotFoundException_ at all

_UsernameNotFoundException_ can be used not only in the authenticate part of a spring application, but in other features, like reset password, in this case only one exception will be likely raised and it should not be _UsernameNotFoundException_ (if the user uses it it is likely to disclose user existence):


----
public String reset_password(String username) throws AuthenticationException {
  Details user = null;
  try {
    user = loadUserByUsername(username);
  } catch (UsernameNotFoundException | DataAccessException e) {
    thrown UsernameNotFoundException("username not found");
  }
}
----

endif::env-github,rspecator-view[]