0

I am new in java, a self learner. I came accross the following issue and was stuck. In fact I am trying to sanitize this code against command injection but failed to understand how. I know how to sanitize user input but this specific has to do with command executed in the OS and I am not sure how anyone help please. here is the code:

public class CommandProcessor {

    public CommandProcessor() {
        // TODO Auto-generated constructor stub
    }
    
    public int invokeCommand(String command) throws  IOException{
        int exitCode =0;
        
        if(command !=null && !command.isEmpty()) {
            Process process = null;
            try {
                process = Runtime.getRuntime().exec(command);
                process.waitFor();
                exitCode = process.exitValue();
                
            }catch(InterruptedException e) {
                
            }
        }
        return exitCode;
        
    }
}
Turing85
  • 18,217
  • 7
  • 33
  • 58
JavaBeginner
  • 3
  • 1
  • 2
    If you allow a remote user to execute an OS command, you open yourself to all sorts of mischief. The only safe way to do this is to NOT do it. It is not clear what you are asking. – Jim Garrison Apr 11 '22 at 18:35
  • You need to narrow your command types ... – LHA Apr 11 '22 at 18:36
  • 1
    TL;DR: You probably can't. The *best* solution is to provide a limited shell or a sandboxed VM. Naive approaches (e.g., checking for specific commands) is prone to all sorts of shell shenanigans. – Dave Newton Apr 11 '22 at 18:37
  • have a known list of commands you will allow and disallow all others. Right now you are allowing anything and everything. – RAZ_Muh_Taz Apr 11 '22 at 18:37
  • This code is not vulnerable to command injection because nothing's being injected anywhere. Any such vulnerability would be in the methods *callling* `invokeCommand`. – that other guy Apr 11 '22 at 18:40
  • Let's say I need to sanitize the String "command" provided by the user before it is executed in the command OS, can I just sanitize it as a regular String or should I take care of the fact that it has to be executed in process.Runtime.getRuntime().exec(command); – JavaBeginner Apr 11 '22 at 19:26

1 Answers1

1

The correct answer is to read the documentation as your current code is not safe.

https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/Runtime.html#exec(java.lang.String%5B%5D)

The "command to execute" should be a constant.

Jonathan S. Fisher
  • 8,189
  • 6
  • 46
  • 84