3

Small example, reduced from a REST API node.js app:

const { exec } = require('child_process');
var userInput = 'untrusted source';
var cmd = `/bin/echo "${userInput}"`;
exec(cmd, function(err, stdout, stderr) {
    console.log('echo: ' + stdout);
});

Assuming the userInput is from an untrusted source, what needs to be done avoid any vulnerability? For example, the quoted "${userInput}" parameter for echo avoids input 'evil spirit; rm -rf /' from causing damage. What else needs to be done to stay safe?

Update: The objective is to make a few existing shell scripts/commands in the file system available via a REST API on the intranet.

Peter Thoeny
  • 7,379
  • 1
  • 10
  • 20
  • Escape all backslashes and quotation marks and you'll have a mostly OK but still naive solution – Klaycon May 11 '20 at 21:18
  • Don't run untested code. – Get Off My Lawn May 11 '20 at 21:24
  • @Kalycon: Yes, backticks are obvious, as in 'untrusted \`date\`' – Peter Thoeny May 11 '20 at 21:26
  • @GetOffMyLawn: "don't run untested code"/"use a VM" is not helpful in this case. The objective is to make existing specific shell scripts/commands on the intranet available via a REST API (I updated the post for clarification) – Peter Thoeny May 11 '20 at 21:31
  • Found related question: https://stackoverflow.com/questions/49512370/sanitize-user-input-for-child-process-exec-command – Peter Thoeny May 11 '20 at 22:02
  • Based on the docs at https://nodejs.org/api/child_process.html#child_process_child_process_spawn_command_args_options it is safer to use child_process.spawn(). This is to avoid the shell, where metacharacters may be used to trigger arbitrary command execution – Peter Thoeny May 11 '20 at 22:33

1 Answers1

1

Based on the official Node.js child_process doc at https://nodejs.org/api/child_process.html#child_process_child_process_spawn_command_args_options it is (obviously) unsafe to use user input in shell scripts without sanitizing it:

If the shell option is enabled, do not pass unsanitized user input to this function. Any input containing shell metacharacters may be used to trigger arbitrary command execution.

So, here is the example stated in my question, rewritten in a safe way using spawn instead of exec:

const { spawn } = require('child_process');

var userInput = 'untrusted source';
var args = [ userInput ];
var cmd = '/bin/echo';
var subprocess = spawn(cmd, args);
var stderr = '';
var stdout = '';
subprocess.stdout.on('data', function(data) {
    stdout += data;
});
subprocess.stderr.on('data', function(data) {
    stderr += data;
});
subprocess.on('close', function(exitCode) {
  console.log('echo: ' + stdout);
});

This is a simplified code snippet of a CLI wrapper Node.js app that make existing commands and shell scripts on an internal network available in a secure way via a REST API: https://github.com/peterthoeny/rest-cli-io

Peter Thoeny
  • 7,379
  • 1
  • 10
  • 20