javasecurity:S5145 false positives

Versions:

  • SonarQube, version 9.1.0.47736
  • Maven Sonar plugin 3.9.0.2155

I have two example of false positives.

Example 1 - Session variables:

    @GetMapping(POLICY_DETAILS_PERFORM)
    public @ResponseBody
    Messages getPolicyDetails(@ModelAttribute(ModelKeys.SELECTED_POLICY) final String policyNumber, @SessionAttribute(SessionKeys.USER) final User user) {

logger.debug(REQUEST_POLICY_NUMBER_USER_MESSAGE, policyNumber, user);

SonarQube is warning about logging the user session variable. This isn’t user-controlled data.

Example 2 - @Valid annotation:

    @PostMapping(value = REGISTRATION_PERFORM,consumes = "application/json")
    public @ResponseBody Messages registerUser(@Valid @RequestBody RegistrationForm form, BindingResult result)

SonarQube is warning about logging form.getXXX() properties. They’ve already been validated so it should be safe to log them.

Hello @rriopel, and welcome to our community :grinning:

Thanks for the report! I will look into this and come back to you in the next few days.

Hello @rriopel! And I apologise for being so late.

Thanks again for your report! If you and I are sure that the “User” and “RegistrationForm” objects comply with AppSec best practises, we can report the issues you reported as false positives.

Example 1: Session variables
In a typical user registration, users can specify their “user” attributes such as “username”, which makes these attributes attack vectors.
Can you prove that the actual users of your code absolutely cannot register “user” string attributes, or that this data is sanitised?

Example 2 - @Valid annotation
Can you provide me with the “RegistrationForm” schema used for validation by the “@Valid” annotation for confirmation?

To answer these questions, I suggest you answer with code samples. I find it best to check both examples for correct type validation, type masks (e.g. for strings), regex validation patterns and size checking.

Note: To avoid code leaks, I recommend anonymising the names in your code before sharing it.

Thanks!

Loris

  1. In this case, the user session attribute is loaded from a database using data from Auth0 so it’s definitely valid.

  2. Here’s a subset of the properties of the RegistrationForm class:

    @PolicyNumber(required = true)
    public String policyNumber;
    @NotBlank(field = "Salutation")
    @Length(max = 10, field = "Salutation")
    public String salutation;
    @NotBlank(field = "Owner First Name")
    @Length(max = 15, field = "Owner First Name")
    public String ownerFirstName;
    @NotBlank(field = "Owner Last Name")
    @Length(max = 25, field = "Owner Last Name")
    public String ownerLastName;

Each of these annotations are a validation Constraint (e.g. Constraint(validatedBy = NotBlankValidator.class). In this example, NotBlankValidator.class implements the ConstraintValidator interface.

Does that help? Let me know if you need anything further.