Change this code to not construct the URL from user-controlled data

Must-share information (formatted with Markdown):

  • which versions are you using (SonarQube, Scanner, Plugin, and any relevant extension)
  • Sonar cloud is integrated to azure devops
  • Trying to remove the SSRF security vulnerability

Do not share screenshots of logs – share the text itself (bonus points for being well-formatted)!

        public HttpResponseMessage ActivatePendingRsAwards([FromBody] RSAwardsDto rsAwards)
        {
            if (rsAwards.AwardIds == null && rsAwards.AwardIds.ToList().Exists(x => x < 0))
            {
                var errResult = new SaveResult();
                errResult.Errors.Add("Invalid Award Id");
                return Request.CreateResponse(HttpStatusCode.BadRequest, errResult);
            }
            var result = _awardRepository.ActivatePendingRsAwards(rsAwards.AwardIds, SessionVars.CompanyID, SessionVars.UserID);
            if (result.ToList().TrueForAll(x => x.Successful))
            {
                return Request.CreateResponse(HttpStatusCode.OK, result);
            }
            return Request.CreateResponse(HttpStatusCode.BadRequest, result);
        }

Getting Change this code to not construct the URL from user-controlled data. exception for the above controller method. Validated the rsAwards also but still getting this issue.

Hi,

Can you give a little more information about the context?

  • The code looks like .Net Framework 4 code but I am not sure. Can you confirm it is .Net Framework and not .Net Core? Can you also confirm the version number?
  • Which line raises a SSRF vulnerability and for which rule exactly? Is it S5144?
  • What ActivatePendingRsAwards is doing?
  • “Validated the rsAwards” what do you mean exactly?

It helps if you can provide a reproducer of the issue.

Best regards

Sebastien

1 Like
  • It is a .NET Core code Version 8.0.100.
    image

  • The line return Request.CreateResponse(HttpStatusCode.BadRequest, result); raises the SSRF vulnerability.

  • The ActivatePendingRsAwards method is a controller method which calls the repository method ActivatePendingRsAwards which in turn does a linq query to turn the status of the award ids passed.

  • By validated the rsAwards I meant the if block where I’m doing null check and less than 0 check.

I am not sure it is related, but this code looks odd to me: you are checking that rsAwards.AwardIds is null and you access it member ToList(). So it looks like it does not check anything and in case rsAwards.AwardIds is null, it will crash.

Hi @Dhanush_Krishna,

Are you still having this issue ?

If yes, to better investigate this issue, please bear with me: If you click on your SonarCloud issue, you should see something that looks like the screenshot I took:

Mine has different colors and different style, maybe yours might look like that:

In any case, what’s important for investigation is that we can understand what is the flow of the vulnerability, this means we need to understand from the “Source” (where potential attack data is entered) to the “Sink” (where the vulnerability is triggered), and all the lines in between (where it is written “tainted value is propagated”).

Is it possible for you to share the code that gets highlighted when you click on the “execution flows”? Or maybe take a screenshot of the same thing I took a screenshot of, so that I can guide you afterwards to understand what’s going on.

Also, it should be written roslyn.sonaranalyzer.security.cs:SXXXX under the title, can you tell me what is written on your issue ? The string will also help us move forward with the problem. It should be there:

Thanks,

Kind regards,

Loris

1 Like

Thanks Loris_Sierra . I have also looked into the sonar cloud portal which you have mentioned in the screenshot. Now the issue is fixed.

I did a null check to the rsAwards (parameter of the ActivatePendingRsAwards method). This null check fixed the sonar scan. It has fixed probably because without a null check can cause error in the repository method call, which leads to unexpected behaviour.

2 Likes

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.