pazz/alot — Issue #157: please avoid args = shlex.split(...); subprocess.Popen(args, ...)
Author: teythoon
Created: 2011-12-15 16:22:13+00:00
State: open

What you're doing here is unserializing a string and handing it over to execve. This is worse than just using os.system or Popen(..., shell = True) since it looks like it's a safer variant when in fact it is not.

And this is a problem in practice. Try this: hit 'm' to compose a mail, at the to prompt hit '' and *boom*, ['abook', '--mutt-query'] is executed resulting in abook failing with:

```
abook: option '--mutt-query' requires an argument
```

What happened?

```
resultstring = cmd_output('%s %s' % (self.command, prefix)) # => resultstring = cmd_output('abook --mutt-query ')

in cmd_output:

args = shlex.split(command_line.encode('utf-8', errors='ignore')) # => args = ['abook', '--mutt-query']
```

man execve says:
```
int execve(const char *filename, char *const argv[],
char *const envp[]);
```

argv is an array. Using strings and deserializing them into an array is an error prone process, whether the shell is doing it or shlex. Best to avoid this and to just hand arrays to functions like cmd_output.