From 9819d2fa57c7e829a7c5edd82bb984252a788d95 Mon Sep 17 00:00:00 2001 From: Calin Crisan Date: Wed, 18 May 2016 22:55:05 +0300 Subject: [PATCH] all subprocess calls should now be save (removed shell=True where possible, escaped where not) --- motioneye/config.py | 13 +++++++------ motioneye/diskctl.py | 2 +- motioneye/handlers.py | 9 ++++++--- motioneye/mediafiles.py | 10 +++++----- motioneye/motionctl.py | 2 +- motioneye/powerctl.py | 2 +- motioneye/smbctl.py | 6 +++--- motioneye/tzctl.py | 2 +- motioneye/v4l2ctl.py | 17 +++++++++-------- 9 files changed, 34 insertions(+), 29 deletions(-) diff --git a/motioneye/config.py b/motioneye/config.py index 97b8a9c..906fcbe 100644 --- a/motioneye/config.py +++ b/motioneye/config.py @@ -18,6 +18,7 @@ import collections import datetime import errno +import glob import logging import os.path import re @@ -1388,9 +1389,10 @@ def backup(): if len(os.listdir(settings.CONF_PATH)) > 100: logging.debug('config path "%s" appears to be a system-wide config directory, performing a selective backup' % settings.CONF_PATH) - cmd = 'cd "%s" && tar zc motion.conf thread-*.conf' % settings.CONF_PATH + cmd = ['tar', 'zc', 'motion.conf'] + cmd += map(os.path.basename, glob.glob(os.path.join(settings.CONF_PATH, 'thread-*.conf'))) try: - content = subprocess.check_output(cmd, shell=True) + content = subprocess.check_output(cmd, cwd=settings.CONF_PATH) logging.debug('backup file created (%s bytes)' % len(content)) return content @@ -1403,9 +1405,8 @@ def backup(): else: logging.debug('config path "%s" appears to be a motion-specific config directory, performing a full backup' % settings.CONF_PATH) - cmd = 'cd "%s" && tar zc .' % settings.CONF_PATH try: - content = subprocess.check_output(cmd, shell=True) + content = subprocess.check_output(['tar', 'zc', '.'], cwd=settings.CONF_PATH) logging.debug('backup file created (%s bytes)' % len(content)) return content @@ -1424,10 +1425,10 @@ def restore(content): logging.info('restoring config from backup file') - cmd = 'tar zxC "%s" || true' % settings.CONF_PATH + cmd = ['tar', 'zxC', settings.CONF_PATH] try: - p = subprocess.Popen(cmd, shell=True, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) msg = p.communicate(content)[0] if msg: logging.error('failed to restore configuration: %s' % msg) diff --git a/motioneye/diskctl.py b/motioneye/diskctl.py index b4cd738..aba8f7b 100644 --- a/motioneye/diskctl.py +++ b/motioneye/diskctl.py @@ -150,7 +150,7 @@ def _list_disks_dev_by_id(): def _list_disks_fdisk(): try: - output = subprocess.check_output('fdisk -l 2>/dev/null', shell=True) + output = subprocess.check_output(['fdisk', '-l'], stderr=open('/dev/null', 'w')) except Exception as e: logging.error('failed to list disks using "fdisk -l": %s' % e, exc_info=True) diff --git a/motioneye/handlers.py b/motioneye/handlers.py index 5384a9f..ab79f32 100644 --- a/motioneye/handlers.py +++ b/motioneye/handlers.py @@ -712,6 +712,9 @@ class ConfigHandler(BaseHandler): @BaseHandler.auth(admin=True) def backup(self): content = config.backup() + + if not content: + raise Exception('failed to create backup file') filename = 'motioneye-config.tar.gz' self.set_header('Content-Type', 'application/x-compressed') @@ -1485,7 +1488,7 @@ class ActionHandler(BaseHandler): self.run_command_bg(command) def run_command_bg(self, command): - self.p = subprocess.Popen(command, shell=True, stderr=subprocess.STDOUT, stdout=subprocess.PIPE) + self.p = subprocess.Popen(command, stderr=subprocess.STDOUT, stdout=subprocess.PIPE) self.command = command self.io_loop = IOLoop.instance() @@ -1628,8 +1631,8 @@ class LogHandler(BaseHandler): logging.debug('serving log file "%s" from command "%s"' % (filename, path)) try: - output = subprocess.check_output(path, shell=True) - + output = subprocess.check_output(path.split()) + except Exception as e: output = 'failed to execute command: %s' % e diff --git a/motioneye/mediafiles.py b/motioneye/mediafiles.py index bafc62d..6f2684f 100644 --- a/motioneye/mediafiles.py +++ b/motioneye/mediafiles.py @@ -154,7 +154,7 @@ def _remove_older_files(dir, moment, exts): def find_ffmpeg(): try: - return subprocess.check_output('which ffmpeg', stderr=open('/dev/null'), shell=True).strip() + return subprocess.check_output(['which', 'ffmpeg'], stderr=open('/dev/null', 'w')).strip() except subprocess.CalledProcessError: # not found return None @@ -210,12 +210,12 @@ def make_movie_preview(camera_config, full_path): logging.debug('creating movie preview for %(path)s with an offset of %(offs)s seconds...' % { 'path': full_path, 'offs': offs}) - cmd = 'ffmpeg -i "%(path)s" -f mjpeg -vframes 1 -ss %(offs)s -y %(path)s.thumb' + cmd = 'ffmpeg -i %(path)s -f mjpeg -vframes 1 -ss %(offs)s -y %(path)s.thumb' actual_cmd = cmd % {'path': full_path, 'offs': offs} logging.debug('running command "%s"' % actual_cmd) try: - subprocess.check_output(actual_cmd, shell=True, stderr=subprocess.STDOUT) + subprocess.check_output(actual_cmd.split(), stderr=subprocess.STDOUT) except subprocess.CalledProcessError as e: logging.error('failed to create movie preview for %(path)s: %(msg)s' % { @@ -239,7 +239,7 @@ def make_movie_preview(camera_config, full_path): # try again, this time grabbing the very first frame try: - subprocess.check_output(actual_cmd, shell=True, stderr=subprocess.STDOUT) + subprocess.check_output(actual_cmd.split(), stderr=subprocess.STDOUT) except subprocess.CalledProcessError as e: logging.error('failed to create movie preview for %(path)s: %(msg)s' % { @@ -560,7 +560,7 @@ def make_timelapse_movie(camera_config, framerate, interval, group): } logging.debug('executing "%s"' % cmd) - + _timelapse_process = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, shell=True) _timelapse_process.progress = 0.01 # 1% diff --git a/motioneye/motionctl.py b/motioneye/motionctl.py index 741294a..0ed7f5c 100644 --- a/motioneye/motionctl.py +++ b/motioneye/motionctl.py @@ -51,7 +51,7 @@ def find_motion(): else: # autodetect motion binary path try: - binary = subprocess.check_output('which motion', stderr=open('/dev/null'), shell=True).strip() + binary = subprocess.check_output(['which', 'motion'], stderr=open('/dev/null', 'w')).strip() except subprocess.CalledProcessError: # not found return None diff --git a/motioneye/powerctl.py b/motioneye/powerctl.py index 3b82ab7..403cb49 100644 --- a/motioneye/powerctl.py +++ b/motioneye/powerctl.py @@ -22,7 +22,7 @@ import subprocess def _find_prog(prog): try: - return subprocess.check_output('which %s' % prog, shell=True).strip() + return subprocess.check_output(['which', prog], stderr=open('/dev/null', 'w')).strip() except subprocess.CalledProcessError: # not found return None diff --git a/motioneye/smbctl.py b/motioneye/smbctl.py index 201c1da..4281378 100644 --- a/motioneye/smbctl.py +++ b/motioneye/smbctl.py @@ -39,7 +39,7 @@ def stop(): def find_mount_cifs(): try: - return subprocess.check_output('which mount.cifs', shell=True).strip() + return subprocess.check_output(['which', 'mount.cifs'], stderr=open('/dev/null', 'w')).strip() except subprocess.CalledProcessError: # not found return None @@ -164,7 +164,7 @@ def _mount(server, share, username, password): try: logging.debug('mounting "//%s/%s" at "%s" (sec=%s)' % (server, share, mount_point, sec)) - subprocess.check_call('mount.cifs "//%s/%s" "%s" -o "%s"' % (server, share, mount_point, actual_opts), shell=True) + subprocess.check_call(['mount.cifs', '//%s/%s' % (server, share), mount_point, '-o', actual_opts]) break except subprocess.CalledProcessError: @@ -194,7 +194,7 @@ def _umount(server, share, username): logging.debug('unmounting "//%s/%s" from "%s"' % (server, share, mount_point)) try: - subprocess.check_call('umount "%s"' % mount_point, shell=True) + subprocess.check_call(['umount', mount_point]) except subprocess.CalledProcessError: logging.error('failed to unmount smb share "//%s/%s" from "%s"' % (server, share, mount_point)) diff --git a/motioneye/tzctl.py b/motioneye/tzctl.py index d06ed23..a757273 100644 --- a/motioneye/tzctl.py +++ b/motioneye/tzctl.py @@ -61,7 +61,7 @@ def _get_time_zone_md5(): return None try: - output = subprocess.check_output('cd /usr/share/zoneinfo; find * -type f | xargs md5sum', shell=True) + output = subprocess.check_output('find * -type f | xargs md5sum', shell=True, cwd='/usr/share/zoneinfo') except Exception as e: logging.error('getting md5 of zoneinfo files failed: %s' % e) diff --git a/motioneye/v4l2ctl.py b/motioneye/v4l2ctl.py index 027ef55..77bb33f 100644 --- a/motioneye/v4l2ctl.py +++ b/motioneye/v4l2ctl.py @@ -18,6 +18,7 @@ import fcntl import logging import os.path +import pipes import re import stat import subprocess @@ -34,7 +35,7 @@ _DEV_V4L_BY_ID = '/dev/v4l/by-id/' def find_v4l2_ctl(): try: - return subprocess.check_output('which v4l2-ctl', shell=True).strip() + return subprocess.check_output(['which', 'v4l2-ctl'], stderr=open('/dev/null', 'w')).strip() except subprocess.CalledProcessError: # not found return None @@ -48,7 +49,7 @@ def list_devices(): try: output = '' started = time.time() - p = subprocess.Popen('v4l2-ctl --list-devices 2>/dev/null', shell=True, stdout=subprocess.PIPE, bufsize=1) + p = subprocess.Popen(['v4l2-ctl', '--list-devices'], stdout=subprocess.PIPE, bufsize=1) fd = p.stdout.fileno() fl = fcntl.fcntl(fd, fcntl.F_GETFL) @@ -120,8 +121,8 @@ def list_resolutions(device): resolutions = set() output = '' started = time.time() - p = subprocess.Popen('v4l2-ctl -d "%(device)s" --list-formats-ext | grep -vi stepwise | grep -oE "[0-9]+x[0-9]+" || true' % { - 'device': device}, shell=True, stdout=subprocess.PIPE, bufsize=1) + p = subprocess.Popen('v4l2-ctl -d %(device)s --list-formats-ext | grep -vi stepwise | grep -oE "[0-9]+x[0-9]+" || true' % { + 'device': pipes.quote(device)}, shell=True, stdout=subprocess.PIPE, bufsize=1) fd = p.stdout.fileno() fl = fcntl.fcntl(fd, fcntl.F_GETFL) @@ -321,8 +322,8 @@ def _set_ctrl(device, control, value): output = '' started = time.time() - p = subprocess.Popen('v4l2-ctl -d "%(device)s" --set-ctrl %(control)s=%(value)s' % { - 'device': device, 'control': control, 'value': value}, shell=True, stdout=subprocess.PIPE, bufsize=1) + p = subprocess.Popen('v4l2-ctl -d %(device)s --set-ctrl %(control)s=%(value)s' % { + 'device': pipes.quote(device), 'control': pipes.quote(control), 'value': pipes.quote(str(value))}, shell=True, stdout=subprocess.PIPE, bufsize=1) fd = p.stdout.fileno() fl = fcntl.fcntl(fd, fcntl.F_GETFL) @@ -366,8 +367,8 @@ def _list_ctrls(device): output = '' started = time.time() - p = subprocess.Popen('v4l2-ctl -d "%(device)s" --list-ctrls' % { - 'device': device}, shell=True, stdout=subprocess.PIPE, bufsize=1) + p = subprocess.Popen('v4l2-ctl -d %(device)s --list-ctrls' % { + 'device': pipes.quote(device)}, shell=True, stdout=subprocess.PIPE, bufsize=1) fd = p.stdout.fileno() fl = fcntl.fcntl(fd, fcntl.F_GETFL) -- 2.39.5