Potential Null Dereference during the execution of command

[rng70-or]

Potential Null Dereference Path

In file: CommandShellExecutor.java there is the following code segment

public static <V, E> CommandShellResultSet<V, E> executeCommand(String command, Function<CommandShellResultSet<List<String>, List<String>>, CommandShellResultSet<V, E>> mapper) {
        return executeCommand(command).map(mapper);
}

which invokes another method executeCommand

public static CommandShellResultSet<List<String>, List<String>> executeCommand(String command) {
        String[] fullCommand = computeCommand(command);
        return execute(fullCommand);
}

which invokes execute

private static CommandShellResultSet<List<String>, List<String>> execute(String[] fullCommand) {
        CommandShellResultSet<List<String>, List<String>> resultSet = null;
        try {
            Process process = Runtime.getRuntime().exec(fullCommand);
            List<String> value = readOutput(process, Process::getInputStream);
            List<String> errors = readOutput(process, Process::getErrorStream);
            resultSet = new CommandShellResultSet<>(value, errors);
        } catch (IOException e) {
            LoggingService.logError(MODULE_NAME, e.getMessage(), e);
        }
        return resultSet;
}

this method initialize the resultSet variable with null value and only assign any value to it inside ta try-catch block. But before assigning any value to resultSet if IOException occurs due to readOutput or Runtime.getRuntime().exec() then resultSet will be returned as null which can later lead to NullPointerException in the very first method due to null.map(mapper)

Possible Workaround

One possible workaround is always check the returned value of execute() like

public static <V, E> CommandShellResultSet<V, E> executeCommand(String command, Function<CommandShellResultSet<List<String>, List<String>>, CommandShellResultSet<V, E>> mapper) {
        CommandShellResultSet<List<String>, List<String>> resultSet = executeCommand(command);
        if(resultSet != null){
            return resultSet.map(mapper);
        }else{
            // return something appropriate
        }
}

another workaround can be use of try-catch which is generally not recommended as a best practice

public static <V, E> CommandShellResultSet<V, E> executeCommand(String command, Function<CommandShellResultSet<List<String>, List<String>>, CommandShellResultSet<V, E>> mapper) {
        try{
            return executeCommand(command).map(mapper);
        }catch(NullPointerException e){
            // do something appropriate
        }finally{
            // if needed
        }
}

as the executeCommand method was used in the several places in the codebase so, if any of the places this scenario arises which can lead to an unexpected behavior or crash the program.

Fix from triage team!!!

We have mentioned to return something appropriate because of lack of knowledge of the codebase, it is not possible to know which value should be the appropriate value to return in case of any null vlaue or which fix type is more appropriate for this codebase.

Sponsorship and Support:

This work is done by the security researchers from OpenRefactory and is supported by the Open Source Security Foundation (OpenSSF): Project Alpha-Omega. Alpha-Omega is a project partnering with open source software project maintainers to systematically find new, as-yet-undiscovered vulnerabilities in open source code - and get them fixed - to improve global software supply chain security.

The bug is found by running the iCR tool by OpenRefactory, Inc. and then manually triaging the results.