passing user input to command line

Question

I am using crack lib to check password strength for my web application. However, I’m concerned about whether or not my current implementation is secure.

The user enters a password, which is then their run through the following node.js code:

      var checkPassword = exec('echo "'+password+ '"| cracklib-check\n', function(err, stdout, stderr) {
     if (stderr) {
        reject(stderr);
     } else if (err) {
        reject(err);
     } else if (stdout) {
        resolve(stdout);            
     } else {
        reject('Password Validation Failed');
     }
  });

because the password is being concatenated into the command, it seems like an attack as possible, Kind of like SQL injection. Is this approach secure, and if not, how do I fix it? If I were to blacklist double quotes, would that avoid all potential issues?

Did you mean to wrap that in quotes? As you wrote it, there are no changes to the file system. However, if you wrap that string with double quotes, it adds a file to the filesystem. — PunDefeated, Oct 27 '16 at 00:01
yes, that could cause problems. can you place it in a temp file and load it from there? — dandavis, Oct 27 '16 at 03:25
i mean to put the password in a temp file so that it's not a literal on the shell — dandavis, Oct 27 '16 at 18:38

score 1 · Accepted Answer · answered Oct 27 '16 at 20:43

because the password is being concatenated into the command, it seems like an attack as possible, Kind of like SQL injection.

Indeed, it's very easy to do a "; rm -rf / ;"-type attack.

If I were to blacklist double quotes, would that avoid all potential issues?

It's far more secure to avoid passing commands through a shell at all. You should instead be using the exec family of functions (or rather, an appropriate wrapper around them in your language of choice), where you pass a pre-parsed command; this is much more failsafe than trying to implement a shell quoting blacklist. Most language's wrappers will also provide an easy way to pass stdin to the command, which is the only thing you're using multiple processes and a pipeline for.

You still need to consider other possibilities of attack (for instance, does cracklib-check consume a lot of resources when passed a very long string? Then you've opened yourself up to a classic Denial of Service attack), but it will at least avoid the injection-type holes.

Do you know of any node libraries that are able to handle that task? — PunDefeated, Oct 28 '16 at 16:52
I'm not familiar with them, but [`spawn`](https://nodejs.org/api/child_process.html#child_process_child_process_spawn_command_args_options) and [`spawnSync`](https://nodejs.org/api/child_process.html#child_process_child_process_spawnsync_command_args_options) look like the standard type. You want to make sure you don't set the `shell` parameter to true. — Xiong Chiamiov, Oct 28 '16 at 19:12

passing user input to command line

1 Answers1