CVE-2021-23418: Improper Restriction of XML External Entity Reference (CWE-611)
value: The package glances before 3.2.1 are vulnerable to XML External Entity (XXE) Injection via the use of Fault to parse untrusted XML data, which is known to be vulnerable to XML attacks.
CWE description: The product processes an XML document that can contain XML entities with URIs that resolve to documents outside of the intended sphere of control, causing the product to embed incorrect documents into its output.
CVSS v3 base score: 9.8
Published: 2021-07-29T18:15Z
Repository: https://github.com/nicolargo/glances
Language: Python
Files changed: glances/actions.py, glances/config.py, glances/secure.py, unitest.py
Commit date: 2021-04-24 16:14:35 +0200
Commit message: Security audit - B411 #1025

Diff: diff --git a/glances/actions.py b/glances/actions.py
index ea94a816..02b4724f 100644
--- a/glances/actions.py
+++ b/glances/actions.py
@@ -17,14 +17,13 @@
 # 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/>.
 
 """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
 except ImportError:
     logger.debug("Chevron library not found (action scripts won't work)")
@@ -92,15 +91,19 @@ class GlancesActions(object):
                 cmd_full = cmd
             # Execute the action
             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)
 
         return True
diff --git a/glances/config.py b/glances/config.py
index 2e001b65..e8053019 100644
--- a/glances/config.py
+++ b/glances/config.py
@@ -291,11 +291,11 @@ class Config(object):
         """Get the value of an option, if it exists.
 
         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
         try:
             ret = self.parser.get(section, option)
diff --git a/glances/secure.py b/glances/secure.py
new file mode 100644
index 00000000..7a2f8d7b
--- /dev/null
+++ b/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
diff --git a/unitest.py b/unitest.py
index ea91ea9e..0ed66d5b 100755
--- a/unitest.py
+++ b/unitest.py
@@ -33,15 +33,15 @@ from glances.thresholds import GlancesThresholdCareful
 from glances.thresholds import GlancesThresholdWarning
 from glances.thresholds import GlancesThresholdCritical
 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()
 
 # Init Glances stats
 stats = GlancesStats(config=core.get_config(),
@@ -374,10 +374,16 @@ class TestGlances(unittest.TestCase):
         bar.percent = -1
         self.assertLessEqual(bar.percent, bar.min_value)
         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')
         stats.end()
         self.assertTrue(True)

Vulnerable code: from subprocess import Popen

            logger.debug("Action will be executed with the following command: \
                subprocess.Popen({}, shell=False)".format(cmd_full.split(' ')))
                Popen(cmd_full.split(' '), shell=False)
                logger.error("Can't execute the action ({})".format(e))
        Dynamic vlaue should starts and end with the ` char

Fixed code: from glances.secure import secure_popen
                ret = secure_popen(cmd_full)
                logger.error("Action error for {} ({}): {}".format(stat_name,
                                                                   criticity,
                                                                   e))
            else:
                logger.debug("Action result for {} ({}): {}".format(stat_name,
                                                                    criticity, 
                                                                    ret))
        Dynamic value should starts and end with the ` char
# -*- 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
from glances.secure import secure_popen
    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')