'SonnarQube's issue: change code to not construct the URL from user-controlled data

I facing a SonarQube bug and am not able to figure out whats the issue. SonnarQube's issue is, change this code to not construct the URL from user-controlled data.

@Value("${...}")
String apiKey;

@Value("${...}")
String apiUrl;

public Response apiResponse(String location) {

   HttpHeaders headers = new HttpHeaders();
   headers.add("x-apikey", apiKey);

   HttpEntity<Object> entity = new HttpEntity<>(headers);

   String url = apiUrl + location; // SonarQube issue: tainted value is propagated

   Response response = null;

   try {

      ResponseEntity<Response> responseEntity = restTemplate.exchange(url, HttpMethod.GET, entity, Response.class); // SonarQube issue: Tainted value is used to perform a security- sensitive operation.

      response = responseEntity.getBody();

   } catch(Exception){

       // doesn't throw anything

   }

   return response;

}

@Cacheable(...)
Response cacheResponse(String location, String tokenKey) {

  return apiResponse(location); // SonarQube issue: tainted value is propagated
}

This fixed the issue, but why is that so? and how can I apply this in the above code?

 String url = apiUrl + location; // SonarQube issue: tainted

Instead, I just tried hardcoding the value of location and fixed the issue.

String url = apiUrl + "location";

So weird...

javaspringspring-bootsonarqube


Solution 1:[1]

I added validation for the Location variable and this solved the issue

if(!location.matches(...)) {
   throw error.....
}

String url = apiUrl + location;

Solution 2:[2]

What SonarQube is trying to tell you is that you are exposing your logic to input from the clients. A better solution would be to refactor your code to not depend on a specific header from the client to perform some action. Its hard to suggest sample code without seeing a little more of the codebase.

Solution 3:[3]

You are using input from the client/user (namly in the variable location) to construct an URL. So if the client/user supplies an malicious value to location he could form an invalid URL.

In the second example String url = apiUrl + "location"; you are not using user input, as "location" is a hard coded String.

I don't know what you try to achieve with the code. But maybe it's better to hold a list of possible URLs and the user supplies and enum value that maps to an URL.

Solution 4:[4]

String url = "https://someurl/%s";
url = String.format(url,location);
sendRequest(url);

Maybe this approach won't give error.

Sources

This article follows the attribution requirements of Stack Overflow and is licensed under CC BY-SA 3.0.

Source: Stack Overflow

Solution Source
Solution 1 ima.technophyle
Solution 2 Dharman
Solution 3 TomStroemer
Solution 4

Related Questions