CVE-2016-6902: Permissions, Privileges, and Access Controls (CWE-264)
value: lshell 0.9.16 allows remote authenticated users to break out of a limited shell and execute arbitrary commands.
CWE description: Weaknesses in this category are related to the management of permissions, privileges, and other security features that are used to perform access control.
CVSS v3 base score: 9.9
Published: 2017-04-24T19:59Z
Repository: https://github.com/ghantoos/lshell
Language: Python
Files changed: lshell/sec.py, test/test_functional.py, test/test_unit.py
Commit date: 2016-08-25 01:13:37 -0400
Commit message: [security] parse quoted strings for possible commands (Closes #148, Closes #147, Closes #149)

Both issues #148 and #147 use the same vulnerability in the parser,
that ignored the quoted strings. Parsing only the rest of the line
for security issues. This is a major security bug.

This commits also corrects a previous ommited correction regarding the
control charaters, that permitted to escape from lshell.

Thank you Proskurin Kirill (@Oloremo) and Vladislav Yarmak (@Snawoot)
for reporting this!!

Diff: diff --git a/lshell/sec.py b/lshell/sec.py
index ea3561f..ff9557c 100644
--- a/lshell/sec.py
+++ b/lshell/sec.py
@@ -172,16 +172,13 @@ def check_secure(line, conf, strict=None, ssh=None):
     for item in relist:
         if os.path.exists(item):
             ret_check_path, conf = check_path(item, conf, strict=strict)
             returncode += ret_check_path
 
-    # ignore quoted text
-    line = re.sub(r'\"(.+?)\"', '', line)
-    line = re.sub(r'\'(.+?)\'', '', line)
-
-    if re.findall('[:cntrl:].*\n', line):
-        ret, conf = warn_count('syntax',
+    # parse command line for control characters, and warn user
+    if re.findall(r'[\x01-\x1F\x7F]', oline):
+        ret, conf = warn_count('control char',
                                oline,
                                conf,
                                strict=strict,
                                ssh=ssh)
         return ret, conf
diff --git a/test/test_functional.py b/test/test_functional.py
index d139e7e..5d3b1d5 100644
--- a/test/test_functional.py
+++ b/test/test_functional.py
@@ -382,25 +382,43 @@ class TestFunctions(unittest.TestCase):
         self.child.expect('%s:~\$' % self.user)
         result = self.child.before.decode('utf8').split('\n')[1].strip()
 
         self.assertEqual(expected, result)
 
-    def test_28_catch_lnext_terminal_ctrl(self):
-        """ F25 | test ctrl-v ctrl-j then command, forbidden/security """
+    def test_28_catch_terminal_ctrl_j(self):
+        """ F28 | test ctrl-v ctrl-j then command, forbidden/security """
         self.child = pexpect.spawn('%s/bin/lshell '
                                    '--config %s/etc/lshell.conf '
                                    % (TOPDIR, TOPDIR))
         self.child.expect('%s:~\$' % self.user)
 
-        expected = u'*** forbidden syntax: echo\r'
+        expected = u'*** forbidden control char: echo\r'
         self.child.send('echo')
         self.child.sendcontrol('v')
         self.child.sendcontrol('j')
         self.child.sendline('bash')
         self.child.expect('%s:~\$' % self.user)
 
         result = self.child.before.decode('utf8').split('\n')
 
         self.assertIn(expected, result)
 
+    def test_29_catch_terminal_ctrl_k(self):
+        """ F29 | test ctrl-v ctrl-k then command, forbidden/security """
+        self.child = pexpect.spawn('%s/bin/lshell '
+                                   '--config %s/etc/lshell.conf '
+                                   % (TOPDIR, TOPDIR))
+        self.child.expect('%s:~\$' % self.user)
+
+        expected = u'*** forbidden control char: echo\x0b() bash && echo\r'
+        self.child.send('echo')
+        self.child.sendcontrol('v')
+        self.child.sendcontrol('k')
+        self.child.sendline('() bash && echo')
+        self.child.expect('%s:~\$' % self.user)
+
+        result = self.child.before.decode('utf8').split('\n')[1]
+
+        self.assertIn(expected, result)
+
 if __name__ == '__main__':
     unittest.main()
diff --git a/test/test_unit.py b/test/test_unit.py
index 137f551..d5ed417 100644
--- a/test/test_unit.py
+++ b/test/test_unit.py
@@ -16,20 +16,10 @@ TOPDIR = '%s/../' % os.path.dirname(os.path.realpath(__file__))
 class TestFunctions(unittest.TestCase):
     args = ['--config=%s/etc/lshell.conf' % TOPDIR, "--quiet=1"]
     userconf = CheckConfig(args).returnconf()
     shell = ShellCmd(userconf, args)
 
-    def test_01_checksecure_doublequote(self):
-        """ U01 | quoted text should not be forbidden """
-        INPUT = 'ls -E "1|2" tmp/test'
-        return self.assertEqual(sec.check_secure(INPUT, self.userconf)[0], 0)
-
-    def test_02_checksecure_simplequote(self):
-        """ U02 | quoted text should not be forbidden """
-        INPUT = "ls -E '1|2' tmp/test"
-        return self.assertEqual(sec.check_secure(INPUT, self.userconf)[0], 0)
-
     def test_03_checksecure_doublepipe(self):
         """ U03 | double pipes should be allowed, even if pipe is forbidden """
         args = self.args + ["--forbidden=['|']"]
         userconf = CheckConfig(args).returnconf()
         INPUT = "ls || ls"
@@ -219,7 +209,22 @@ class TestFunctions(unittest.TestCase):
         args = self.args + ["--allowed=['echo','export']", "--path_noexec=''"]
         userconf = CheckConfig(args).returnconf()
         # verify that no alias was created containing LD_PRELOAD
         return self.assertNotIn('echo', userconf['aliases'])
 
+    def test_26_checksecure_quoted_command(self):
+        """ U26 | quoted command should be parsed """
+        INPUT = 'echo 1 && "bash"'
+        return self.assertEqual(sec.check_secure(INPUT, self.userconf)[0], 1)
+
+    def test_27_checksecure_quoted_command(self):
+        """ U27 | quoted command should be parsed """
+        INPUT = '"bash" && echo 1'
+        return self.assertEqual(sec.check_secure(INPUT, self.userconf)[0], 1)
+
+    def test_28_checksecure_quoted_command(self):
+        """ U28 | quoted command should be parsed """
+        INPUT = "echo'/1.sh'"
+        return self.assertEqual(sec.check_secure(INPUT, self.userconf)[0], 1)
+
 if __name__ == "__main__":
     unittest.main()

Vulnerable code:     # ignore quoted text
    line = re.sub(r'\"(.+?)\"', '', line)
    line = re.sub(r'\'(.+?)\'', '', line)

    if re.findall('[:cntrl:].*\n', line):
        ret, conf = warn_count('syntax',
    def test_28_catch_lnext_terminal_ctrl(self):
        """ F25 | test ctrl-v ctrl-j then command, forbidden/security """
        expected = u'*** forbidden syntax: echo\r'
    def test_01_checksecure_doublequote(self):
        """ U01 | quoted text should not be forbidden """
        INPUT = 'ls -E "1|2" tmp/test'
        return self.assertEqual(sec.check_secure(INPUT, self.userconf)[0], 0)

    def test_02_checksecure_simplequote(self):
        """ U02 | quoted text should not be forbidden """
        INPUT = "ls -E '1|2' tmp/test"
        return self.assertEqual(sec.check_secure(INPUT, self.userconf)[0], 0)

Fixed code:     # parse command line for control characters, and warn user
    if re.findall(r'[\x01-\x1F\x7F]', oline):
        ret, conf = warn_count('control char',
    def test_28_catch_terminal_ctrl_j(self):
        """ F28 | test ctrl-v ctrl-j then command, forbidden/security """
        expected = u'*** forbidden control char: echo\r'
    def test_29_catch_terminal_ctrl_k(self):
        """ F29 | test ctrl-v ctrl-k then command, forbidden/security """
        self.child = pexpect.spawn('%s/bin/lshell '
                                   '--config %s/etc/lshell.conf '
                                   % (TOPDIR, TOPDIR))
        self.child.expect('%s:~\$' % self.user)

        expected = u'*** forbidden control char: echo\x0b() bash && echo\r'
        self.child.send('echo')
        self.child.sendcontrol('v')
        self.child.sendcontrol('k')
        self.child.sendline('() bash && echo')
        self.child.expect('%s:~\$' % self.user)

        result = self.child.before.decode('utf8').split('\n')[1]

        self.assertIn(expected, result)

    def test_26_checksecure_quoted_command(self):
        """ U26 | quoted command should be parsed """
        INPUT = 'echo 1 && "bash"'
        return self.assertEqual(sec.check_secure(INPUT, self.userconf)[0], 1)

    def test_27_checksecure_quoted_command(self):
        """ U27 | quoted command should be parsed """
        INPUT = '"bash" && echo 1'
        return self.assertEqual(sec.check_secure(INPUT, self.userconf)[0], 1)

    def test_28_checksecure_quoted_command(self):
        """ U28 | quoted command should be parsed """
        INPUT = "echo'/1.sh'"
        return self.assertEqual(sec.check_secure(INPUT, self.userconf)[0], 1)