# Code seems to be wrong in compliant solution

**URL:** <https://community.sonarsource.com/t/code-seems-to-be-wrong-in-compliant-solution/56413>\
**Category:** SonarQube Cloud\
**Tags:** csharp, security, sonarqube-cloud\
**Created:** [January 14, 2022, 4:19pm UTC](https://community.sonarsource.com/t/code-seems-to-be-wrong-in-compliant-solution/56413 "2022-01-14T16:19:40Z")\
**Posts on this page:** 9\
**Page:** 1

<div class="post-metadata">

**Author:** ![jeff-yaeger](https://dub1.discourse-cdn.com/sonarsource/user_avatar/community.sonarsource.com/jeff-yaeger/32/29242_2.png) [@jeff-yaeger](https://community.sonarsource.com/u/jeff-yaeger)\
**Post date:** [January 14, 2022, 4:19pm UTC](https://community.sonarsource.com/t/code-seems-to-be-wrong-in-compliant-solution/56413/1 "2022-01-14T16:19:40Z")

</div>

Template for a good bug report, formatted with [Markdown](https://commonmark.org/help/):

- Versions used (SonarQube, Scanner, Plugin, and any relevant extension)
- Error observed (wrap logs/code around triple quote ``` for proper formatting)
- Steps to reproduce
- Potential workaround
- Scanner command used when applicable (private details masked)
- In case of SonarCloud:
  - ALM used (GitHub, Bitbucket Cloud, Azure DevOps)
  - CI system used (Bitbucket Cloud, Azure DevOps, Travis CI, Circle CI, Jenkins, other)

Why is this an issue →   
I/O function calls should not be vulnerable to path injection attacks

```auto
 public class RSPEC2083IOInjectionCompliantController : Controller
    {
        public IActionResult Index()
        {
            return View();
        }

        public IActionResult DeleteFile(string fileName)
        {
            string destDirectory = "~/CustomersData/";

            string destFileName = Path.GetFullPath(System.IO.Path.Combine(destDirectory, fileName));
            string fullDestDirPath = Path.GetFullPath(destDirectory + Path.DirectorySeparatorChar);

            if (!destFileName.StartsWith(fullDestDirPath, StringComparison.Ordinal)) <--- Should remove ! in front of destFileName
            {
                System.IO.File.Delete(destFileName); // Compliant
                return Content("File " + fileName + " deleted");
            } else
            {
                return BadRequest();
            }
        }

    }

```

---

<div class="post-metadata">

**Author:** ![cba](https://avatars.discourse-cdn.com/v4/letter/c/ecb155/32.png) [@cba](https://community.sonarsource.com/u/cba)\
**Post date:** [January 14, 2022, 5:49pm UTC](https://community.sonarsource.com/t/code-seems-to-be-wrong-in-compliant-solution/56413/2 "2022-01-14T17:49:52Z")

</div>

Hi @jeff-yaeger ,

Welcome to the community!

You copied the template for a good bug report, but then did not fill in any of the details. It’s important for us to know as much as possible about your environment to help with relevant solutions.

As for your code: what happens when you pass a filename such as “…/…/foo.sys”? Now you’re outside of the application directory and can delete any file you wish, assuming the application runs with enough privileges to delete that file.

---

<div class="post-metadata">

**Author:** ![jeff-yaeger](https://dub1.discourse-cdn.com/sonarsource/user_avatar/community.sonarsource.com/jeff-yaeger/32/29242_2.png) [@jeff-yaeger](https://community.sonarsource.com/u/jeff-yaeger)\
**Post date:** [January 14, 2022, 5:57pm UTC](https://community.sonarsource.com/t/code-seems-to-be-wrong-in-compliant-solution/56413/3 "2022-01-14T17:57:15Z")

</div>

Hi @cba ,

I’m pretty sure this should read:

```auto
if (destFileName.StartsWith(fullDestDirPath, StringComparison.Ordinal)) 
            {
                System.IO.File.Delete(destFileName); // Compliant
                return Content("File " + fileName + " deleted");
            } else
            {
                return BadRequest();
            }

```

Because you want to validate that the path you are creating starts with the destination path you expect.  
The way it’s written currently, it would return a BadRequest for having the correct file path.  
Just wanted to point that out.

---

<div class="post-metadata">

**Author:** ![cba](https://avatars.discourse-cdn.com/v4/letter/c/ecb155/32.png) [@cba](https://community.sonarsource.com/u/cba)\
**Post date:** [January 14, 2022, 6:23pm UTC](https://community.sonarsource.com/t/code-seems-to-be-wrong-in-compliant-solution/56413/4 "2022-01-14T18:23:48Z")

</div>

Hi @jeff-yaeger ,

Are you 100% sure that Path.GetFullPath will always collapse a path like `C:\app\CustomersData\..\..\Windows\foo.sys` into `C:\Windows\foo.sys` ? Even if someone has fun e.g. with UTF8 characters for directory traversal?

In a string comparison, `C:\app\CustomersData\..\..\Windows\foo.sys` still starts with the correct path, so will still delete the system file. That’s all that SonarQube is trying to tell you - review your code as there may be unintended side effects. If you’re confident that your code is secure, then you can resolve this issue as “Won’t fix”.

---

<div class="post-metadata">

**Author:** ![jeff-yaeger](https://dub1.discourse-cdn.com/sonarsource/user_avatar/community.sonarsource.com/jeff-yaeger/32/29242_2.png) [@jeff-yaeger](https://community.sonarsource.com/u/jeff-yaeger)\
**Post date:** [January 14, 2022, 8:25pm UTC](https://community.sonarsource.com/t/code-seems-to-be-wrong-in-compliant-solution/56413/5 "2022-01-14T20:25:43Z")

</div>

Hi @cba, that’s not what I’m trying to say at all. I understand the reason for checking the full path. I’m saying with the example code put out by sonar cube, it sends you to the wrong result. If the path is correct, it returns a BadRequest, if the path doesn’t start with the fullDestDirPath(i.e. the path is wrong), it deletes and returns a message saying the file is deleted.

---

<div class="post-metadata">

**Author:** ![cba](https://avatars.discourse-cdn.com/v4/letter/c/ecb155/32.png) [@cba](https://community.sonarsource.com/u/cba)\
**Post date:** [January 14, 2022, 9:26pm UTC](https://community.sonarsource.com/t/code-seems-to-be-wrong-in-compliant-solution/56413/6 "2022-01-14T21:26:27Z")

</div>

Ah I see. That’s where filling out the template details would have helped 😉

Which version of SonarQube is this? Or is this SonarCloud? And what is the rule name? I’ll flag this post so that some of the more experienced folks can take a closer look.

---

<div class="post-metadata">

**Author:** ![jeff-yaeger](https://dub1.discourse-cdn.com/sonarsource/user_avatar/community.sonarsource.com/jeff-yaeger/32/29242_2.png) [@jeff-yaeger](https://community.sonarsource.com/u/jeff-yaeger)\
**Post date:** [January 14, 2022, 11:08pm UTC](https://community.sonarsource.com/t/code-seems-to-be-wrong-in-compliant-solution/56413/7 "2022-01-14T23:08:01Z")

</div>

@cba Your right, I should have used the template. This is sonar cloud. The name of the rule is “I/O function calls should not be vulnerable to path injection attacks”.

---

<div class="post-metadata">

**Author:** ![Pierre-Loup\_Tristant](https://dub1.discourse-cdn.com/sonarsource/user_avatar/community.sonarsource.com/pierre-loup_tristant/32/3191_2.png) [@Pierre-Loup\_Tristant](https://community.sonarsource.com/u/Pierre-Loup_Tristant)\
**Post date:** [January 28, 2022, 4:07pm UTC](https://community.sonarsource.com/t/code-seems-to-be-wrong-in-compliant-solution/56413/10 "2022-01-28T16:07:30Z")

</div>

Hi @jeff-yaeger and welcome to the community.

Thanks for reported this problem.  
The change should be visible in a few weeks.

---

<div class="post-metadata">

**Author:** ![system](https://dub1.discourse-cdn.com/sonarsource/user_avatar/community.sonarsource.com/system/32/31083_2.png) [@system](https://community.sonarsource.com/u/system)\
**Post date:** [February 4, 2022, 4:07pm UTC](https://community.sonarsource.com/t/code-seems-to-be-wrong-in-compliant-solution/56413/12 "2022-02-04T16:07:56Z")

</div>

Closed on February 4, 2022, 4:07pm UTC

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