From a536c1dc4f4c28da96a64a6c91e6ed5061e3c077 Mon Sep 17 00:00:00 2001 From: Florent Le Coz Date: Mon, 22 Oct 2012 17:14:21 +0200 Subject: Make the Executor class reliable. Plugins do not need to escape the command arguments or remove the line breaks and care about how the will get parsed anymore, they just need to pass a list of args. Do not spawn an additional shell, for more clarity, simplicity and possibly security. --- plugins/link.py | 4 +--- plugins/simple_notify.py | 17 +++++++++-------- src/core.py | 47 ++++++++++++++++++++++++++++++++--------------- src/daemon.py | 21 +++++++++++++++++++-- 4 files changed, 61 insertions(+), 28 deletions(-) diff --git a/plugins/link.py b/plugins/link.py index 8d757be6..427d718a 100644 --- a/plugins/link.py +++ b/plugins/link.py @@ -7,7 +7,6 @@ from plugin import BasePlugin from xhtml import clean_text import common import tabs -import pipes url_pattern = re.compile(r'\b(http[s]?://(?:\S+))\b', re.I|re.U) @@ -42,8 +41,7 @@ class Plugin(BasePlugin): nb = 1 link = self.find_link(nb) if link: - link = pipes.quote(link) - self.core.exec_command("%s %s" % (self.config.get('browser', 'firefox'), link)) + self.core.exec_command([self.config.get('browser', 'firefox'), link]) else: self.core.information('No URL found.', 'Warning') diff --git a/plugins/simple_notify.py b/plugins/simple_notify.py index c2cbb198..d274e0ee 100644 --- a/plugins/simple_notify.py +++ b/plugins/simple_notify.py @@ -1,7 +1,7 @@ from plugin import BasePlugin from xhtml import clean_text, get_body_from_message_stanza from timed_events import DelayedEvent -import pipes +import shlex class Plugin(BasePlugin): def init(self): @@ -25,14 +25,15 @@ class Plugin(BasePlugin): body = clean_text(get_body_from_message_stanza(message)) if not body: return - command = self.config.get('command', '').strip() - if not command: + command_str = self.config.get('command', '').strip() + if not command_str: self.core.information('No notification command was provided in the configuration file', 'Warning') return - self.core.exec_command(command % {'body':pipes.quote(body), 'from':pipes.quote(fro)}) - after_command = self.config.get('after_command', '').strip() - if not after_command: + command = [arg % {'body': body.replace('\n', ' '), 'from': fro} for arg in shlex.split(command_str)] + self.core.exec_command(command) + after_command_str = self.config.get('after_command', '').strip() + if not after_command_str: return - delayed_event = DelayedEvent(self.config.get('delay', 1), self.core.exec_command, after_command % {'body':pipes.quote(body), 'from':pipes.quote(fro)}) + after_command = [arg % {'body': body.replace('\n', ' '), 'from': fro} for arg in shlex.split(after_command_str)] + delayed_event = DelayedEvent(self.config.get('delay', 1), self.core.exec_command, after_command) self.core.add_timed_event(delayed_event) -4 diff --git a/src/core.py b/src/core.py index bd83c51f..9609ec44 100644 --- a/src/core.py +++ b/src/core.py @@ -11,6 +11,7 @@ import os import sys import time import curses +import pipes import ssl from functools import reduce @@ -492,17 +493,32 @@ class Core(object): def exec_command(self, command): """ - Execute an external command on the local or a remote - machine, depending on the conf. For example, to open a link in a - browser, do exec_command("firefox http://poezio.eu"), - and this will call the command on the correct computer. - The remote execution is done by writing the command on a fifo. - That fifo has to be on the machine where poezio is running, and - accessible (through sshfs for example) from the local machine (where - poezio is not running). A very simple daemon reads on that fifo, - and executes any command that is read in it. - """ - command = '%s\n' % (command,) + Execute an external command on the local or a remote machine, + depending on the conf. For example, to open a link in a browser, do + exec_command(["firefox", "http://poezio.eu"]), and this will call + the command on the correct computer. + + The command argument is a list of strings, not quoted or escaped in + any way. The escaping is done here if needed. + + The remote execution is done + by writing the command on a fifo. That fifo has to be on the + machine where poezio is running, and accessible (through sshfs for + example) from the local machine (where poezio is not running). A + very simple daemon (daemon.py) reads on that fifo, and executes any + command that is read in it. Since we can only write strings to that + fifo, each argument has to be pipes.quote()d. That way the + shlex.split on the reading-side of the daemon will be safe. + + You cannot use a real command line with pipes, redirections etc, but + this function supports a simple case redirection to file: if the + before-last argument of the command is ">" or ">>", then the last + argument is considered to be a filename where the command stdout + will be written. For example you can do exec_command(["echo", + "coucou les amis coucou coucou", ">", "output.txt"]) and this will + work. If you try to do anything else, your |, [, <<, etc will be + interpreted as normal command arguments, not shell special tokens. + """ if config.get('exec_remote', 'false') == 'true': # We just write the command in the fifo if not self.remote_fifo: @@ -511,16 +527,17 @@ class Core(object): except (OSError, IOError) as e: self.information('Could not open fifo file for writing: %s' % (e,), 'Error') return + command_str = ' '.join([pipes.quote(arg.replace('\n', ' ')) for arg in command]) + '\n' try: - self.remote_fifo.write(command) + self.remote_fifo.write(command_str) except (IOError) as e: - self.information('Could not execute [%s]: %s' % (command.strip(), e,), 'Error') + self.information('Could not execute %s: %s' % (command, e,), 'Error') self.remote_fifo = None else: - e = Executor(command.strip()) + e = Executor(command) try: e.start() - except ValueError as e: # whenever shlex fails + except ValueError as e: self.information('%s' % (e,), 'Error') diff --git a/src/daemon.py b/src/daemon.py index 5d8c9fab..08cf480f 100755 --- a/src/daemon.py +++ b/src/daemon.py @@ -38,17 +38,34 @@ class Executor(threading.Thread): def __init__(self, command): threading.Thread.__init__(self) self.command = command + # check for > or >> special case + self.filename = None + self.redirection_mode = 'w' + if len(command) >= 3: + if command[-2] in ('>', '>>'): + self.filename = command.pop(-1) + if command[-1] == '>>': + self.redirection_mode = 'a' + command.pop(-1) def run(self): log.info('executing %s' % (self.command,)) - subprocess.call(['sh', '-c', self.command]) + stdout = None + if self.filename: + try: + stdout = open(self.filename, self.redirection_mode) + except (OSError, IOError) as e: + log.error('Could not open redirection file: %s (%s)' % (self.filename, e,)) + return + subprocess.call(self.command, stdout=stdout) def main(): while True: line = sys.stdin.readline() if line == '': break - e = Executor(line) + command = shlex.split(line) + e = Executor(command) e.start() if __name__ == '__main__': -- cgit v1.2.3