cshekar641719 (Community Member) asked a question.

June 8, 2020 at 11:25 AM
what's causing an OS Command injection(CWE-78) flaw in the following c# code

According to recommendation of CWE-78, my function below has been validated user input, but Veracode still reports that CWE-78 is available in that function. 

 

 private static void DisplayReport(string fileName)

    {

      var p = new Process();

      var pi = new ProcessStartInfo {FileName = FilePathCleanser(fileName) };

      p.StartInfo = pi;

      p.Start();

    }

    private static string FilePathCleanser(string filename)

    {

      if (!string.IsNullOrEmpty(filename) && File.Exists(filename) && Path.GetFileNameWithoutExtension(filename).All(char.IsLetterOrDigit))

        return filename;

      return null;

    }

  • 2 answers
  • 2.11K views
Milo likes this.

  • Anthony Fielding (Veracode)

    Hi @cshekar641719 (Community Member)​,

    Veracode Static Analysis is unable to verify the changes you have made sufficiently remove the risk from taking untrusted user input and executing it. The resolution for each instance is contextual to how the application processes the untrusted input, and so you will always be required to propose a mitigation. Such proposal should clearly state all the controls in place to mitigate the risk, such as from your example the following:

    • The file must exist.
    • The file name without it's extension must be only letters or digits.

     

    The mitigation should also detail any unaddressed, outstanding risk. I can see there is validation in place, but there are some questions I would have around this implementation:

    • Is it possible the extension could contain something malicious since the extension is not being validated. Could it contain unexpected characters that could affect the flow of operation?
    • What if the fileName is a system executable, could it be possible that by executing an arbitrary binary found on the host affect the security - e.g. shutdown.exe?
    • What if the fileName is a UNC path to a remote server under the attacker's control?

     

    Where possible it is always best to set the executable from a known and hard-coded list of allowed options that the application is in control of, rather than allow the user to specify the executable.

     

    It is also a good idea to fully resolve the path using Path.GetFullPath() and verify the executable lives in a known and expected path, e.g. assert that the fully resolved path starts with "C:\App\Tools\".

     

    Please also review this article which could assist you with the recommended validations required: https://community.veracode.com/s/article/how-do-i-fix-cwe-73-external-control-of-file-name-or-path-in-java.

     

    I hope that helps.

     

    Thanks,

    Anthony Fielding

    Expand Post

Topics (4)

No articles found
Loading

Ask the Community

Get answers, share a use case, discuss your favorite features, or get input from the community.