<p>We're currently building an application that executes a number of external tools.  We often have to pass information entered into our system by users to these tools.</p>

<p>Obviously, this is a big security nightmare waiting to happen.</p>

<p>Unfortunately, we've not yet found any classes in the .NET Framework that execute command line programs while providing the same kind of guards against injection attacks as the IDbCommand objects do for databases.</p>

<p>Right now, we're using a very primitive string substitution which I suspect is rather insufficient:</p>

<blockquote>
<pre><code>protected virtual string Escape(string value)
{
      return value
        .Replace(@"\", @"\\")
        .Replace(@"$", @"\$")
        .Replace(@"""", @"\""")
        .Replace("`", "'")
      ;
}
</code></pre>
</blockquote>

<p>What do you guys do to prevent command-line injection attacks?  We're planning to implement a regex that is very strict and only allows a very small subset of characters through, but I was wondering if there was a better way.</p>

<p>Some clarifications:</p>

<ul>
<li>Some of these tools do not have APIs we can program against.  If they did, we wouldn't be having this problem.</li>
<li>The users don't pick tools to execute, they enter meta-data which the tools we've chosen use (for example, injecting meta data such as copyright notices into target files). </li>
</ul>

[{'answer_id': 44807, 'author': 'Curt Hagenlocher', 'author_id': 533, 'author_profile': 'https://Stackoverflow.com/users/533', 'pm_score': 4, 'selected': True, 'text': "<p>Are you executing the programs directly or going through the shell?  If you always launch an external program by giving the full path name to the executable and leaving the shell out of the equation, then you aren't really susceptible to any kind of command line injection.</p>\n\n<p>EDIT: DrFloyd, the shell is responsible for evaluating things like the backtick.  No shell, no shell evaluation.  Obviously, you've still got to be aware of any potential security gotchas in the programs that you're calling -- but I don't think this question is about that.</p>\n"}, {'answer_id': 44813, 'author': 'Mike Stone', 'author_id': 122, 'author_profile': 'https://Stackoverflow.com/users/122', 'pm_score': 0, 'selected': False, 'text': "<p>Well, if you can invoke the tools programmatically without the command line, that would probably be your best option.  Otherwise, you could potentially execute the command line tool via a user that has absolutely no access to do anything (except perhaps a single directory that they can't do any harm with)... though that may end up breaking the tool, depending on what the tool does.</p>\n\n<p>Just note, I have never had to face this problem, because I have never actually had to invoke a command line tool from an externally facing application where the tool requires input from the user.</p>\n"}, {'answer_id': 44823, 'author': 'DrFloyd5', 'author_id': 1736623, 'author_profile': 'https://Stackoverflow.com/users/1736623', 'pm_score': 0, 'selected': False, 'text': '<p>Hmmm...</p>\n\n<p>It sounds like you have a list of valid commands that the users are able to execute. But you don\'t want them to execute them all.</p>\n\n<p>You could try to take the actual command line and verify the file exists in the "safe" location at least.</p>\n\n<p>You could also solve the problem with more interface, provide a drop down of commands and parameters they could use. It\'s more work on your end, but it ultimately helps the users.</p>\n'}, {'answer_id': 44832, 'author': 'DrFloyd5', 'author_id': 1736623, 'author_profile': 'https://Stackoverflow.com/users/1736623', 'pm_score': 0, 'selected': False, 'text': '<blockquote>\n  <p>Are you executing the programs directly or going through the shell? If you always launch an external program by giving the full path name to the executable and leaving the shell out of the equation, then you aren\'t really susceptible to any kind of command line injection.</p>\n</blockquote>\n\n<p><strong>@Curt Hagenlocher</strong> The backtick can kill you. If the Windows System is setup "wrong", or the unix system allows it, a dir &bt;del *&bt; will first execute the del * command then use the output  in place of the del *, which in this case, won\'t matter because there is nothing to dir (or ls)</p>\n'}, {'answer_id': 44840, 'author': 'chakrit', 'author_id': 3055, 'author_profile': 'https://Stackoverflow.com/users/3055', 'pm_score': 2, 'selected': False, 'text': '<p>When you <a href="http://msdn.microsoft.com/en-us/library/system.diagnostics.process.start(VS.71).aspx" rel="nofollow noreferrer">Process.Start</a> a new process, supply the parameters in its Parameters argument instead of building the whole command line yourself.</p>\n\n<p>Haven\'t got time for a proper test, but I think that should help guard it to some level.</p>\n\n<p>Will test this out tomorrow.</p>\n\n<p><strong>EDIT:</strong> Ah, someone beat me to it again. But here\'s another point: Try using the Console.InputStream (can\'t remember exact name) to supply data instead of passing parameters, is that a possible solution? like fix the command so it reads from the CON device and you then supply the data via input stream instead.</p>\n'}, {'answer_id': 48832, 'author': 'Shadow2531', 'author_id': 1697, 'author_profile': 'https://Stackoverflow.com/users/1697', 'pm_score': 2, 'selected': False, 'text': '<p>In C++ on <strong>Windows</strong>, you just escape \\ and " where needed, quote the argument and ShellExecute it. Then, everything inside the quotes should be treated as text.</p>\n\n<p>This should illustrate:</p>\n\n<pre><code>\n#include &lt;iostream&gt;\n#include &lt;string&gt;\n#include &lt;windows.h&gt;\n#include &lt;cstdlib&gt;\nusing namespace std;\n\n// Escape and quote string for use as Windows command line argument\nstring qEscape(const string& s) {\n    string result("\\"");\n    for (string::const_iterator i = s.begin(); i != s.end(); ++i) {\n        const char c = *i;\n        const string::const_iterator next = i + 1;\n        if (c == \'"\' || (c == \'\\\\\' && (next == s.end() || *next == \'"\'))) {\n            result += \'\\\\\';\n        }\n        result += c;\n    }\n    result += \'"\';\n    return result;\n}\n\nint main() {\n    // Argument value to pass: c:\\program files\\test\\test.exe\n    const string safe_program = qEscape("c:\\\\program files\\\\test\\\\test.exe");\n    cout &lt;&lt; safe_program &lt;&lt; " ";\n\n    // Argument value to pass: You\'re the "best" around.\n    const string safe_arg0 = qEscape("You\'re the \\"best\\" around.");\n\n    // Argument value to pass: "Nothing\'s" gonna ever keep you down.\n    const string safe_arg1 = qEscape("\\"Nothing\'s\\" gonna ever keep you down.");\n\n    const string safe_args = safe_arg0 + " " + safe_arg1;\n    cout &lt;&lt; safe_args &lt;&lt; "\\n\\n";\n\n    // c:\\program files\\test\\  to pass.\n    const string bs_at_end_example = qEscape("c:\\\\program files\\\\test\\\\");\n    cout &lt;&lt; bs_at_end_example &lt;&lt; "\\n\\n";\n\n    const int result = reinterpret_cast&lt;int&gt;(ShellExecute(NULL, "open", safe_program.c_str(), safe_args.c_str(), NULL, SW_SHOWNORMAL));\n    if (result &lt; 33) {\n        cout &lt;&lt; "ShellExecute failed with Error code " &lt;&lt; result &lt;&lt; "\\n";\n        return EXIT_FAILURE;\n    }\n}\n</code></pre>\n\n<p>But, with any method you use, you should test the hell out of it to see that it does really prevent injection.</p>\n'}, {'answer_id': 229780, 'author': 'craigb', 'author_id': 18590, 'author_profile': 'https://Stackoverflow.com/users/18590', 'pm_score': 2, 'selected': False, 'text': "<p>Don't use a blacklist for preventing injections. If there are <em>n</em> ways to inject code, you'll think of <em>n - m</em> where <em>m > 0</em>.</p>\n\n<p>Use a whitelist of accepted parameters (or patterns). It is much more restrictive by nature, but that's the nature of security.</p>\n"}]
['https://Stackoverflow.com/questions/44799', 'https://Stackoverflow.com', 'https://Stackoverflow.com/users/1931/']