DK207186 (Community Member) asked a question.
I am currently working on resolving an OS command injection flaw in Java code. The flaw is at ProcessBuilder's start() method. Here ProcessBuilder List constructor is used. The problem is the content of the List is not checked/validated to prevent OS command injection flaw. So, I validated the List to not to contain certain set of characters which are invalid for the current command. I have also changed the constructor to use String[]. But still Veracode is showing OS command injection flaw. Could someone please help in what point am I missing here?
Old Code:
private static boolean myfun(List<String> args) {
try {
logger.info("Executing: " + args);
ProcessBuilder pb = new ProcessBuilder(args);
Process process = pb.start(); //Veracode command line injection flaw is here
<rest of the code>
New Code:
private static boolean myfun(List<String> args) {
try {
logger.info("Executing: " + args);
if (args.get(0).endsWith("myscript.bat") || args.get(0).endsWith("myscript")) {
logger.info("Executing: " + args);
String cmd = argsarry[0];
args.remove(0);
String onlyargs = String.join(",", args);
ProcessBuilder pb = new ProcessBuilder(cmd, onlyargs);
Process process = pb.start();
<rest of the code>
.png){kind=logo}
Hi @DK207186 (Community Member) ,
Veracode Static Analysis will report CWE 78 Improper Neutralization of Special Elements used in an OS Command (OS Command Injection) if it can detect that there is data from outside of the application (HTTP Request, File, Database, webservice, etc.) being used in a command or it's arguments.
We recommend hardcoding allowed inputs and using only hardcoded data in OS commands executed by the application.
If this is not possible and you must allow some dynamic inputs, then we recommend adding strict whitelist validation and proposing a mitigation ( https://help.veracode.com/reader/DGHxSJy3Gn3gtuSIN2jkRQ/~p4MSKOS8F8X8h0KwFTKoQ ) on this flaw from the Veracode Platform or IDE plugin describing the controls you have in place to prevent an attacker from abusing this functionality and contacting your security team to have these proposals reviewed.
Please let me know if you have any remaining questions or concerns.
Thank you,
Boy Baukema
Thanks @Boy, Security Consultant (Veracode). We must allow dynamic inputs. So we will add validation and mitigate the flaw. Thanks for the response.