Skip to content
Permalink
Browse files Browse the repository at this point in the history
Security audit - B411 #1025
  • Loading branch information
nicolargo committed Apr 24, 2021
1 parent 62efd67 commit 85d5a6b
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 8 deletions.
15 changes: 9 additions & 6 deletions glances/actions.py
Expand Up @@ -19,10 +19,9 @@

"""Manage on alert actions."""

from subprocess import Popen

from glances.logger import logger
from glances.timer import Timer
from glances.secure import secure_popen

try:
import chevron
Expand Down Expand Up @@ -94,12 +93,16 @@ def run(self, stat_name, criticity, commands, repeat, mustache_dict=None):
logger.info("Action triggered for {} ({}): {}".format(stat_name,
criticity,
cmd_full))
logger.debug("Action will be executed with the following command: \
subprocess.Popen({}, shell=False)".format(cmd_full.split(' ')))
try:
Popen(cmd_full.split(' '), shell=False)
ret = secure_popen(cmd_full)
except OSError as e:
logger.error("Can't execute the action ({})".format(e))
logger.error("Action error for {} ({}): {}".format(stat_name,
criticity,
e))
else:
logger.debug("Action result for {} ({}): {}".format(stat_name,
criticity,
ret))

self.set(stat_name, criticity)

Expand Down
2 changes: 1 addition & 1 deletion glances/config.py
Expand Up @@ -293,7 +293,7 @@ def get_value(self, section, option,
If it did not exist, then return the default value.
It allows user to define dynamic configuration key (see issue#1204)
Dynamic vlaue should starts and end with the ` char
Dynamic value should starts and end with the ` char
Example: prefix=`hostname`
"""
ret = default
Expand Down
73 changes: 73 additions & 0 deletions glances/secure.py
@@ -0,0 +1,73 @@
# -*- coding: utf-8 -*-
#
# This file is part of Glances.
#
# Copyright (C) 2021 Nicolargo <nicolas@nicolargo.com>
#
# Glances is free software; you can redistribute it and/or modify
# it under the terms of the GNU Lesser General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# Glances is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU Lesser General Public License for more details.
#
# You should have received a copy of the GNU Lesser General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

"""Secures functions for Glances"""

from glances.compat import nativestr
from subprocess import Popen, PIPE


def secure_popen(cmd):
"""A more or less secure way to execute system command
Return: the result of the command OR an error message
"""
ret = None

# Split by redirection '>'
cmd_split_redirect = cmd.split('>')
if len(cmd_split_redirect) > 2:
return 'Glances error: Only one file redirection allowed ({})'.format(cmd)
elif len(cmd_split_redirect) == 2:
stdout_redirect = cmd_split_redirect[1].strip()
cmd = cmd_split_redirect[0]
else:
stdout_redirect = None

sub_cmd_stdin = None
p_last = None
# Split by pipe '|'
for sub_cmd in cmd.split('|'):
# Split by space ' '
sub_cmd_split = [i for i in sub_cmd.split(' ') if i]
p = Popen(sub_cmd_split,
shell=False,
stdin=sub_cmd_stdin,
stdout=PIPE,
stderr=PIPE)
if p_last is not None:
# Allow p_last to receive a SIGPIPE if p exits.
p_last.stdout.close()
p_last = p
sub_cmd_stdin = p.stdout

p_ret = p_last.communicate()

if nativestr(p_ret[1]) == '':
# No error
ret = nativestr(p_ret[0])
if stdout_redirect is not None:
# Write result to redirection file
with open(stdout_redirect, "w") as stdout_redirect_file:
stdout_redirect_file.write(ret)
else:
# Error
ret = nativestr(p_ret[1])

return ret
8 changes: 7 additions & 1 deletion unitest.py
Expand Up @@ -35,11 +35,11 @@
from glances.thresholds import GlancesThresholds
from glances.plugins.glances_plugin import GlancesPlugin
from glances.compat import subsample, range
from glances.secure import secure_popen

# Global variables
# =================


# Init Glances core
core = GlancesMain()

Expand Down Expand Up @@ -376,6 +376,12 @@ def test_099_output_bars_must_be_between_0_and_100_percent(self):
bar.percent = 101
self.assertGreaterEqual(bar.percent, bar.max_value)

def test_100_secure(self):
"""Test secure functions"""
print('INFO: [TEST_100] Secure functions')
self.assertEqual(secure_popen('echo -n TEST'), 'TEST')
self.assertEqual(secure_popen('echo FOO | grep FOO'), 'FOO\n')

def test_999_the_end(self):
"""Free all the stats"""
print('INFO: [TEST_999] Free the stats')
Expand Down

0 comments on commit 85d5a6b

Please sign in to comment.