S5131 - Endpoints security and XSS attacks

[joevin]

Hi everyone,

I’m using SonarQube 8.3.

One “Vulnerabilities” “Blocker” is rising on my projects and poses me a problem.
Its “Refactor this code to not reflect tainted, user-controlled data.” and when I click on “Why is this an issue?” it mentions " Endpoints should not be vulnerable to reflected cross-site scripting (XSS) attacks".

This is related to this rule : [RSPEC-5131] Endpoints should not be vulnerable to reflected cross-site scripting (XSS) attacks - SonarSource

For me this rule is unnecessary and its treatment causes problems.
Prevent XSS vulnerabilities in front-ends contexts is necessary and they need to encode untrusted inputs (including inputs from API backends).

But prevent XSS on the backend API side break things. Solutions proposed are not sustainable :

1. Validate data based on a whitelist
2. Sanitize data
3. Encode data

The first one is impossible if the data is a comment on a blog for example,
The second one abruptly removes data considered dangerous with a frequently incomplete blacklist (high complexity with a partially achieved security objective). Example : jsoup.clean().
The third one changes integrity of the data (replacing < by &lt ; for exemple). Resulting in unpredictable and undesirable behavior. And requires knowing the front-end context. Exemple : ESAPI.encodeForHtml().

This applies to all APIs.

Do you share the same point of view as I do?

Best regards,
Joévin.

[Hendrik_Buchwald]

Hi Joévin,

Do you have any code samples you could share?

If I understand you correctly you JSON-encode your output and thus you don’t think HTML-encoding it makes sense? I would tend to agree. If the content-type is set correctly it should not be exploitable. Unless the client is doing content type sniffing. But even then I do not think HTML-encoding is a good approach here because - as you said - it changes the data.
JSON provides its own way to encode special characters though, e.g. like this: \u003C. If this encoding is used the data inside of the JSON does not change but you are on the safe side even if type sniffing is done.

I would also recommend to update to the current version of SonarQube (9.1) if you have the chance because our analysis - including and especially XSS - has changed drastically since 8.3.

[joevin]

Hello Hendrick,
Thank you for your answer.
The code sample could be as simple as an endpoint on a Spring Boot application:

package fr.bpce.digital.collecte.controller;

import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestParam;

@Controller
public class HelloController {

    @GetMapping(value = "/test/hello")
    public ResponseEntity<String> hello(@RequestParam(value = "name") String name) {

        return new ResponseEntity<>("Hello " + name + " !", HttpStatus.OK);
    }
}

And this code sample will throw the “Refactor this code to not reflect tainted, user-controlled data.” Issue.
When a GET /test/hello will be requested, the Spring Boot application will reply this JSON object:

{
    "name": "Joévin"
}

And by default, the header Content-Type is set to application/json. From there, when a front-end uses this data, it will be aware that it must adapt the data to the context in which it is inserted. Something that Angular/React/Vue does very well. I don’t think content type sniffing vulnerability is still relevant today in 2021 (unless you are using an old browser).
I went through the release note up to 9.1 but I didn’t see any major changes regarding XSS handling on the APIs. Unfortunately I can’t test SonarQube 9.1 easily.

[ranjeet-sys1 (Ranjeet Kumar Suman)]

Hi Joevin,
I have also same issue and I am exhausted totally to solve this issue. could you please help this same above code for code quality check using sonarQube. SonarQube throwing vulnerability issue, please help me in this below code.

@Controller
public class HelloController {

    @GetMapping(value = "/test/hello")
    public ResponseEntity<String> hello(@RequestParam(value = "name") String name) {

        return new ResponseEntity<>("Hello " + name + " !", HttpStatus.OK);
    }
}

[Pierre-Loup_Tristant]

Hi @ranjeet-sys1,

I tested the code snippet and I can confirm that there is a reflected cross-site scripting vulnerability to fix.
If you try this payload you can verify by yourself: https://<domain>/test/hello?name=%3Cscript%3Ealert(%22xss%22)%3C/script%3E.

The content-type header of the response sent by /test/hello endpoint is text/html. It means that the response will be interpreted as HTML code by web browsers. When your endpoint is producing JSON content as a response you should make sure it sets the content-type header to application/json.

@GetMapping(value = "/test/hello", produces="application/json")
public ResponseEntity<String> hello(@RequestParam(value = "name") String name) {

    return new ResponseEntity<>("Hello " + name + " !", HttpStatus.OK);
}

Pierre-Loup

[joevin]

Hi @Pierre-Loup_Tristant and @ranjeet-sys1,

I just wanted to add a slight precision because I have oversimplified a bit my code snippet.
To make it short, I didn’t used a POJO to build the response object. Without this, the content-type header remains on text/html.

This is what the code snippet should have looked like :

package fr.rest.controller;
import fr.rest.dto.UserDto;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.*;

import java.util.HashMap;
import java.util.Map;

@Controller
public class HelloController {
    @GetMapping("/user")
    public ResponseEntity<UserDto> getUser(String name) {
        UserDto user = new UserDto();
        user.setName(name);
        return new ResponseEntity<>(user, HttpStatus.OK);
    }

    @GetMapping(value = "/hello")
    public ResponseEntity<Map<String, String>> sayHello(@RequestParam(value = "name") String name) {
        Map<String, String> map = new HashMap<>();
        map.put("message", "Hello "+name+" !");
        return new ResponseEntity<>(map, HttpStatus.OK);
    }
}

The getUser uses a DTO class :

public class UserDto {
    protected String name;
    public UserDto() {}
    public String getName() { return this.name; }
    public void setName(String name) { this.name = name; }
}

And now, you can test, those two endpoints returns data with content-type: application/json without the need to add , produces="application/json".

SonarQube should not raise a vulnerability here. If it still does, the rule have to be updated IMO.

Joévin.

[joevin]

@Pierre-Loup_Tristant : what about the RSPEC-5131 ?
Does the latest updates takes into account this content type specificities ?

[Loris (Loris Sierra)]

Hello @joevin, hello @ranjeet-sys1,

Here are some answers to try to clarify this thread definitively.

1. The issue related to ResponseBody and ResponseEntity was noticed and discussed in our internal forums back in 2020. The problem has since been fixed in SonarQube. SonarQube 8.3 is from 2020, so it is essential for your business and user experience that you update SonarQube.

2. @ranjeet-sys1 I think that your use case is different from Joévin’s. Please create another post so that we can discuss a potential fix. Else, if we assume that your use-case is to create JSON, the fix provided by Pierre-Loup is relevant (e.g. adding produces="application/json" in the mapping, without having to change types). Thanks to this attribute, you can also change the return type to a @ResponseBody String, and only return a string: the response will be json data.

3. Regarding the XSS: The issues you mention are related to Spring’s HttpMessageConverters. These converters are directly responsible for how the data and content type are returned in your web application’s responses.

For example, for a ResponseBody<String>, the StringHttpMessageConverter is used, which treats the returned data as a simple string and sets a text content-type (if we assume a simple curl command and a generic Spring MVC configuration).

For other objects, other message converters are used. For example, handling of form data is likely to be performed by a FormHttpMessageConverter.

@joevin, in your first example: If a ResponseBody<String> returns something other than regular text-based data and a content type of text/..., it means that global configuration parameters have explicitly changed this. This process depends on multiple factors, some of them being in other parts of your code.
In the contrary, if the attribute produces="application/json" is set, but the return string is not a valid JSON, the result content-type will be different.

In other cases, such as with custom objects, Spring tries to figure out which internally registered HTTP message converters can be used to return data and set a content-type value. Depending on your configuration, the converter might convert the data to XML or JSON, for example. In general, the converters work by deserializing the data returned or received by the code. (I am assuming that you use <mvc:annotation-driven />.)

@joevin, in your second code example: you used a UserDto and a Map as ResponseEntity types in your last snippets. The StringHttpMessageConverter cannot be the converter used to send data in this case, so another converter was used, serialized the data into JSON format and associated a JSON content-type.
Note: If the configuration was different, the returned data could have been XML.

Creating a Map is IMHO the most simple way to send back JSON.

@joevin, in your first post, you said:

The third one changes integrity of the data (replacing < by &lt ; for exemple). Resulting in unpredictable and undesirable behavior. And requires knowing the front-end context. Exemple : ESAPI.encodeForHtml().

Note that when we say “encoding”, we also include Javascript or JSON encoding, not only HTML encoding. This means that the encoding has to be chosen according to its context of use, and that if the right encoder is chosen, no integrity loss will be experienced.
We will need to clarify this in future versions of the rule. Thanks for the feedback.

@joevin, regarding your last question:

Does the latest [S5131] updates takes into account this content type specificities ?

As mentioned above, these peculiarities have been taken into account since 2020. We still need to continuously improve, but you should not experience these false positives when you update SonarQube.

---

I hope my message answers your questions. For this case, a SonarQube upgrade is the (theoretically) simple solution. I recommend you reach out to Sonar’s support if you need help upgrading.

In any case, thank you very much for your feedback. It is very much appreciated!

Loris

[joevin]

Hello @Loris,

Since 11/2021, we upgraded SonarQube from 8.3 to 8.9.3 and I can now confirm that I no longer encounter S5131 on API projects. The remaining alerts are now relevant. I probably wouldn’t have created this topic if I could have identified that changes were being made to this rule (I didn’t found informations about that inside sonar-java).

As stated in my last post, my first example was not representative of reality and was oversimplified. 99% of the time, on API projects, data is serialized to JSON format like in the second example.

For encoding, I know that there is many context to consider, and and that’s precisely why this countermeasure is almost not applicable in a back-end API context : from the API point of view, it’s impossible to know in advance where the data will be placed (mail, web page, style sheet template, javascript code, etc).

Thank you for these quality discussions.

Joévin.

[Loris (Loris Sierra)]

Hello @joevin,

Great!! Thank you very much for your response.

from the API point of view, it’s impossible to know in advance where the data will be placed (mail, web page, style sheet template, javascript code, etc).

Yes… The best scenario would be to agree on an interface with the front-end developers, but unfortunately, we all know that in practice this is not that easy to do.

Have a good day!

Loris