OpenVPN/openvpn — pull request #9 by crass (2013-10-21 08:11:53+00:00)
Title: Properly quote command and arguments passed to system() in down-root.

Suppose the following command:
```openvpn --config config \
--plugin /usr/lib/openvpn/openvpn-plugin-down-root.so "bash -c \"script_type=down cmd\""```

One might suspect that this would eventually call execve with an array like: ```["bash", "-c", "script_type=down cmd"]```. However the array passed would currently be: ```["bash", "-c", "script_type=down", "cmd"]```

What happens is the plugin argument processing will (eventually) pass ```bash -c "script_type=down cmd"``` to parse_line, which will parse the string to an array as ```["bash", "-c", "script_type=down cmd"]``` and pass it as the argv parameter to down-root's openvpn_plugin_open_v1. So far so good.

Then down-root flattens out the array to a single string by joining the elements with a space using build_command_line. So then we get ```bash -c script_type=down cmd```. This will be fed as the argument to system(), which internally runs ```sh -c 'bash -c script_type=down cmd'```. So bash would then run the command ```script_type=down``` with the first argument of its argument array being ```cmd```, not what we had wanted.

The problem is down-root plugin's build_command_line assumes that no quoting of arguments needs to be done. The current implementation can be hacked around to get the semantics we want here, however providing the right escaping requires either luck or knowledge of how down-root is implemented.

This pull request fixes this by assuming that all elements of the array passed to build_command_line require quoting. If no quoting is required, extra quoting will essentially be ignored.

Perhaps a better solution would be to use execvp/execvpe, instead of flattening the the args to a string, only then have them parsed back into arrays. That would be a more invasive change though, and I don't know if there are any cross-platform issues there.

This patch will make a backwards incompatible change for those have hacked around this issue to get it to work. I would suspect that to be few.