javasecurity:S5145 false positives

Rules and Languages Report False-positive / False-negative...
1

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.

2

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.

3

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

4

  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.

5

Any news on this issue? We’ve seen something similar in SonarQube 9.9 LTS

8

Hi @Alix,

With regards to @rriopel’s original cases:

  1. Session attributes are viewed as vulnerable by the engine. This is by design, for the reasons @Loris stated. Although in this case data is loaded from Auth0 and is valid, it is not possible for our engine to infer this currently.
  2. I set up a test project using the code given and indeed, parameters with an @Valid annotation are still viewed as vulnerable. This is a bug, and I will forward this internally.

I am closing this topic, as it is over a year old. If anything is still unclear, please create a new topic (preferably including a code sample) and I will help you there.